From f0465c4a010be36f3e57c740c3ec4522be14311e Mon Sep 17 00:00:00 2001 From: Nat DeFries <42820733+nmdefries@users.noreply.github.com> Date: Thu, 20 Apr 2023 11:59:21 -0400 Subject: [PATCH 01/21] check num args before ... in f --- R/slide.R | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/R/slide.R b/R/slide.R index ea2b93cba..46e8bb5fd 100644 --- a/R/slide.R +++ b/R/slide.R @@ -155,7 +155,24 @@ epi_slide = function(x, f, ..., before, after, ref_time_values, # Check we have an `epi_df` object if (!inherits(x, "epi_df")) Abort("`x` must be of class `epi_df`.") - + + # Check that `f` takes enough args + if (!missing(f) && is.function(f)) { + # We need `args` here to work properly on primitive functions + arg_names = names(formals(args(f))) + if ("..." %in% arg_names) { + # Keep all arg names before `...` + dots_i <- which(arg_names == "...") + arg_names <- arg_names[seq_len(dots_i - 1)] + } + if (length(arg_names) < 2) { + Abort("`f` must take at least 2 arguments", + class="epiprocess__epi_slide__f_must_take_at_least_2_args", + epiprocess__f = f, + epiprocess__arg_names = arg_names) + } + } + # Arrange by increasing time_value x = arrange(x, time_value) From 138cdb0a7bdf3a515ce68850e9e336e1c212b33c Mon Sep 17 00:00:00 2001 From: Nat DeFries <42820733+nmdefries@users.noreply.github.com> Date: Fri, 21 Apr 2023 10:43:00 -0400 Subject: [PATCH 02/21] only warn if dots provided in args --- R/slide.R | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/R/slide.R b/R/slide.R index 46e8bb5fd..62bbe5237 100644 --- a/R/slide.R +++ b/R/slide.R @@ -158,18 +158,26 @@ epi_slide = function(x, f, ..., before, after, ref_time_values, # Check that `f` takes enough args if (!missing(f) && is.function(f)) { - # We need `args` here to work properly on primitive functions + n_mandatory_f_args <- 2 arg_names = names(formals(args(f))) if ("..." %in% arg_names) { # Keep all arg names before `...` dots_i <- which(arg_names == "...") arg_names <- arg_names[seq_len(dots_i - 1)] - } - if (length(arg_names) < 2) { - Abort("`f` must take at least 2 arguments", - class="epiprocess__epi_slide__f_must_take_at_least_2_args", - epiprocess__f = f, - epiprocess__arg_names = arg_names) + + if (length(arg_names) < n_mandatory_f_args) { + Warn(sprintf("`f` only takes %s positional arguments before the `...` args, but %s were expected; this can lead to obtuse errors downstream", length(arg_names), n_mandatory_f_args), + class="epiprocess__epi_slide__f_needs_min_args_before_dots", + epiprocess__f = f, + epiprocess__arg_names = arg_names) + } + } else { + if (length(arg_names) < n_mandatory_f_args) { + Abort(sprintf("`f` must take at least %s arguments", n_mandatory_f_args), + class="epiprocess__epi_slide__f_needs_min_args", + epiprocess__f = f, + epiprocess__arg_names = arg_names) + } } } From e2c815233514acaa1a7a4c20e6f6944813119c1c Mon Sep 17 00:00:00 2001 From: Nat DeFries <42820733+nmdefries@users.noreply.github.com> Date: Fri, 21 Apr 2023 16:29:14 -0400 Subject: [PATCH 03/21] test num args errors and warnings --- tests/testthat/test-epi_slide.R | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/testthat/test-epi_slide.R b/tests/testthat/test-epi_slide.R index eebcc55b9..f7bb50a68 100644 --- a/tests/testthat/test-epi_slide.R +++ b/tests/testthat/test-epi_slide.R @@ -86,3 +86,31 @@ test_that("these doesn't produce an error; the error appears only if the ref tim dplyr::select("geo_value","slide_value_value"), dplyr::tibble(geo_value = c("ak", "al"), slide_value_value = c(2, -2))) # not out of range for either group }) + +test_that("epi_slide alerts if the provided f doesn't take enough args", { + f_xg = function(x, g) dplyr::tibble(value=mean(x$value), count=length(x$value)) + f_xg_dots = function(x, g, ...) dplyr::tibble(value=mean(x$value), count=length(x$value)) + + expect_no_error(epi_slide(grouped, f_xg, before = 1L, ref_time_values = d+1)) + expect_no_warning(epi_slide(grouped, f_xg, before = 1L, ref_time_values = d+1)) + expect_no_error(epi_slide(grouped, f_xg_dots, before = 1L, ref_time_values = d+1)) + expect_no_warning(epi_slide(grouped, f_xg_dots, before = 1L, ref_time_values = d+1)) + + f_x_dots = function(x, ...) dplyr::tibble(value=mean(x$value), count=length(x$value)) + f_dots = function(...) dplyr::tibble(value=c(5), count=c(2)) + f_x = function(x) dplyr::tibble(value=mean(x$value), count=length(x$value)) + f = function() dplyr::tibble(value=c(5), count=c(2)) + + expect_warning(epi_slide(grouped, f_x_dots, before = 1L, ref_time_values = d+1), + regexp = "positional arguments before the `...` args", + class = "epiprocess__epi_slide__f_needs_min_args_before_dots") + expect_warning(epi_slide(grouped, f_dots, before = 1L, ref_time_values = d+1), + regexp = "positional arguments before the `...` args", + class = "epiprocess__epi_slide__f_needs_min_args_before_dots") + expect_error(epi_slide(grouped, f_x, before = 1L, ref_time_values = d+1), + regexp = "`f` must take at least", + class = "epiprocess__epi_slide__f_needs_min_args") + expect_error(epi_slide(grouped, f, before = 1L, ref_time_values = d+1), + regexp = "`f` must take at least", + class = "epiprocess__epi_slide__f_needs_min_args") +}) From 95fc474ad998bbe3d9771dfe1051334a6f5d8fb3 Mon Sep 17 00:00:00 2001 From: Nat DeFries <42820733+nmdefries@users.noreply.github.com> Date: Mon, 24 Apr 2023 10:31:13 -0400 Subject: [PATCH 04/21] use older support for not raising errors in tests `testthat::expect_no_error` and `testthat::expect_no_warning` are new, experimental functions available in the newest version of `testthat`. The old `testthat::expect_error`, etc, functions also support testing that no errors are raised by setting the `regexp` arg to `NA`. Use these functions so we don't need to require the very newest version of `testthat`. --- tests/testthat/test-epi_slide.R | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/testthat/test-epi_slide.R b/tests/testthat/test-epi_slide.R index f7bb50a68..f25ed5d96 100644 --- a/tests/testthat/test-epi_slide.R +++ b/tests/testthat/test-epi_slide.R @@ -91,10 +91,11 @@ test_that("epi_slide alerts if the provided f doesn't take enough args", { f_xg = function(x, g) dplyr::tibble(value=mean(x$value), count=length(x$value)) f_xg_dots = function(x, g, ...) dplyr::tibble(value=mean(x$value), count=length(x$value)) - expect_no_error(epi_slide(grouped, f_xg, before = 1L, ref_time_values = d+1)) - expect_no_warning(epi_slide(grouped, f_xg, before = 1L, ref_time_values = d+1)) - expect_no_error(epi_slide(grouped, f_xg_dots, before = 1L, ref_time_values = d+1)) - expect_no_warning(epi_slide(grouped, f_xg_dots, before = 1L, ref_time_values = d+1)) + # If `regexp` is NA, asserts that there should be no errors/messages. + expect_error(epi_slide(grouped, f_xg, before = 1L, ref_time_values = d+1), regexp = NA) + expect_warning(epi_slide(grouped, f_xg, before = 1L, ref_time_values = d+1), regexp = NA) + expect_error(epi_slide(grouped, f_xg_dots, before = 1L, ref_time_values = d+1), regexp = NA) + expect_warning(epi_slide(grouped, f_xg_dots, before = 1L, ref_time_values = d+1), regexp = NA) f_x_dots = function(x, ...) dplyr::tibble(value=mean(x$value), count=length(x$value)) f_dots = function(...) dplyr::tibble(value=c(5), count=c(2)) From 066fb4e46e6e9fd183dca946ffd7120fc7be2880 Mon Sep 17 00:00:00 2001 From: Nat DeFries <42820733+nmdefries@users.noreply.github.com> Date: Mon, 24 Apr 2023 11:44:38 -0400 Subject: [PATCH 05/21] factor out args check --- R/grouped_epi_archive.R | 5 +++++ R/slide.R | 22 +--------------------- R/utils.R | 30 ++++++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/R/grouped_epi_archive.R b/R/grouped_epi_archive.R index 1c6bd3110..ac093e6fc 100644 --- a/R/grouped_epi_archive.R +++ b/R/grouped_epi_archive.R @@ -220,6 +220,11 @@ grouped_epi_archive = ref_time_values = sort(ref_time_values) } + # Check that `f` takes enough args + if (!missing(f) && is.function(f)) { + check_sufficient_f_args(f) + } + # Validate and pre-process `before`: if (missing(before)) { Abort("`before` is required (and must be passed by name); diff --git a/R/slide.R b/R/slide.R index 62bbe5237..39f55089f 100644 --- a/R/slide.R +++ b/R/slide.R @@ -158,27 +158,7 @@ epi_slide = function(x, f, ..., before, after, ref_time_values, # Check that `f` takes enough args if (!missing(f) && is.function(f)) { - n_mandatory_f_args <- 2 - arg_names = names(formals(args(f))) - if ("..." %in% arg_names) { - # Keep all arg names before `...` - dots_i <- which(arg_names == "...") - arg_names <- arg_names[seq_len(dots_i - 1)] - - if (length(arg_names) < n_mandatory_f_args) { - Warn(sprintf("`f` only takes %s positional arguments before the `...` args, but %s were expected; this can lead to obtuse errors downstream", length(arg_names), n_mandatory_f_args), - class="epiprocess__epi_slide__f_needs_min_args_before_dots", - epiprocess__f = f, - epiprocess__arg_names = arg_names) - } - } else { - if (length(arg_names) < n_mandatory_f_args) { - Abort(sprintf("`f` must take at least %s arguments", n_mandatory_f_args), - class="epiprocess__epi_slide__f_needs_min_args", - epiprocess__f = f, - epiprocess__arg_names = arg_names) - } - } + check_sufficient_f_args(f) } # Arrange by increasing time_value diff --git a/R/utils.R b/R/utils.R index b398ff5b6..c953b385d 100644 --- a/R/utils.R +++ b/R/utils.R @@ -100,6 +100,36 @@ paste_lines = function(lines) { Abort = function(msg, ...) rlang::abort(break_str(msg, init = "Error: "), ...) Warn = function(msg, ...) rlang::warn(break_str(msg, init = "Warning: "), ...) +#' Check that a sliding computation function takes enough args +#' +#' @param f Function; specifies a computation to slide over an `epi_df` or +#' `epi_archive` in `epi_slide` or `epix_slide`. +#' +#' @noRd +check_sufficient_f_args <- function(f) { + n_mandatory_f_args <- 2 + arg_names = names(formals(args(f))) + if ("..." %in% arg_names) { + # Keep all arg names before `...` + dots_i <- which(arg_names == "...") + arg_names <- arg_names[seq_len(dots_i - 1)] + + if (length(arg_names) < n_mandatory_f_args) { + Warn(sprintf("`f` only takes %s positional arguments before the `...` args, but %s were expected; this can lead to obtuse errors downstream", length(arg_names), n_mandatory_f_args), + class="check_sufficient_f_args__f_needs_min_args_before_dots", + epiprocess__f = f, + epiprocess__arg_names = arg_names) + } + } else { + if (length(arg_names) < n_mandatory_f_args) { + Abort(sprintf("`f` must take at least %s arguments", n_mandatory_f_args), + class="check_sufficient_f_args__f_needs_min_args", + epiprocess__f = f, + epiprocess__arg_names = arg_names) + } + } +} + ########## in_range = function(x, rng) pmin(pmax(x, rng[1]), rng[2]) From ed2a5e97d7eee86d54e9e4a839d19c29e98a72e1 Mon Sep 17 00:00:00 2001 From: Nat DeFries <42820733+nmdefries@users.noreply.github.com> Date: Mon, 24 Apr 2023 11:45:37 -0400 Subject: [PATCH 06/21] test slide fn and arg check fn arg warnings --- tests/testthat/test-epi_slide.R | 8 ++++---- tests/testthat/test-epix_slide.R | 29 +++++++++++++++++++++++++++++ tests/testthat/test-utils.R | 31 ++++++++++++++++++++++++++++++- 3 files changed, 63 insertions(+), 5 deletions(-) diff --git a/tests/testthat/test-epi_slide.R b/tests/testthat/test-epi_slide.R index f25ed5d96..c09781f51 100644 --- a/tests/testthat/test-epi_slide.R +++ b/tests/testthat/test-epi_slide.R @@ -104,14 +104,14 @@ test_that("epi_slide alerts if the provided f doesn't take enough args", { expect_warning(epi_slide(grouped, f_x_dots, before = 1L, ref_time_values = d+1), regexp = "positional arguments before the `...` args", - class = "epiprocess__epi_slide__f_needs_min_args_before_dots") + class = "check_sufficient_f_args__f_needs_min_args_before_dots") expect_warning(epi_slide(grouped, f_dots, before = 1L, ref_time_values = d+1), regexp = "positional arguments before the `...` args", - class = "epiprocess__epi_slide__f_needs_min_args_before_dots") + class = "check_sufficient_f_args__f_needs_min_args_before_dots") expect_error(epi_slide(grouped, f_x, before = 1L, ref_time_values = d+1), regexp = "`f` must take at least", - class = "epiprocess__epi_slide__f_needs_min_args") + class = "check_sufficient_f_args__f_needs_min_args") expect_error(epi_slide(grouped, f, before = 1L, ref_time_values = d+1), regexp = "`f` must take at least", - class = "epiprocess__epi_slide__f_needs_min_args") + class = "check_sufficient_f_args__f_needs_min_args") }) diff --git a/tests/testthat/test-epix_slide.R b/tests/testthat/test-epix_slide.R index 9ef2f9afd..587e5913d 100644 --- a/tests/testthat/test-epix_slide.R +++ b/tests/testthat/test-epix_slide.R @@ -348,3 +348,32 @@ test_that("epix_slide with all_versions option works as intended",{ expect_identical(xx1,xx3) # This and * Imply xx2 and xx3 are identical }) + +test_that("epix_slide alerts if the provided f doesn't take enough args", { + f_xg = function(x, g) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) + f_xg_dots = function(x, g, ...) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) + + # If `regexp` is NA, asserts that there should be no errors/messages. + expect_error(epix_slide(xx, f = f_xg, before = 2L), regexp = NA) + expect_warning(epix_slide(xx, f = f_xg, before = 2L), regexp = NA) + expect_error(epix_slide(xx, f = f_xg_dots, before = 2L), regexp = NA) + expect_warning(epix_slide(xx, f = f_xg_dots, before = 2L), regexp = NA) + + f_x_dots = function(x, ...) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) + f_dots = function(...) dplyr::tibble(value=c(5), count=c(2)) + f_x = function(x) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) + f = function() dplyr::tibble(value=c(5), count=c(2)) + + expect_warning(epix_slide(xx, f_x_dots, before = 2L), + regexp = "positional arguments before the `...` args", + class = "check_sufficient_f_args__f_needs_min_args_before_dots") + expect_warning(epix_slide(xx, f_dots, before = 2L), + regexp = "positional arguments before the `...` args", + class = "check_sufficient_f_args__f_needs_min_args_before_dots") + expect_error(epix_slide(xx, f_x, before = 2L), + regexp = "`f` must take at least", + class = "check_sufficient_f_args__f_needs_min_args") + expect_error(epix_slide(xx, f, before = 2L), + regexp = "`f` must take at least", + class = "check_sufficient_f_args__f_needs_min_args") +}) diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index 08b28c97e..56113a02a 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -107,4 +107,33 @@ test_that("enlist works",{ my_list <- enlist(x=1,y=2,z=3) expect_equal(my_list$x,1) expect_true(inherits(my_list,"list")) -}) \ No newline at end of file +}) + +test_that("check_sufficient_f_args alerts if the provided f doesn't take enough args", { + f_xg = function(x, g) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) + f_xg_dots = function(x, g, ...) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) + + # If `regexp` is NA, asserts that there should be no errors/messages. + expect_error(check_sufficient_f_args(f_xg), regexp = NA) + expect_warning(check_sufficient_f_args(f_xg), regexp = NA) + expect_error(check_sufficient_f_args(f_xg_dots), regexp = NA) + expect_warning(check_sufficient_f_args(f_xg_dots), regexp = NA) + + f_x_dots = function(x, ...) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) + f_dots = function(...) dplyr::tibble(value=c(5), count=c(2)) + f_x = function(x) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) + f = function() dplyr::tibble(value=c(5), count=c(2)) + + expect_warning(check_sufficient_f_args(f_x_dots), + regexp = "positional arguments before the `...` args", + class = "check_sufficient_f_args__f_needs_min_args_before_dots") + expect_warning(check_sufficient_f_args(f_dots), + regexp = "positional arguments before the `...` args", + class = "check_sufficient_f_args__f_needs_min_args_before_dots") + expect_error(check_sufficient_f_args(f_x), + regexp = "`f` must take at least", + class = "check_sufficient_f_args__f_needs_min_args") + expect_error(check_sufficient_f_args(f), + regexp = "`f` must take at least", + class = "check_sufficient_f_args__f_needs_min_args") +}) From 1ec35974d6eb915607163133ec5a5adce4f2b845 Mon Sep 17 00:00:00 2001 From: Nat DeFries <42820733+nmdefries@users.noreply.github.com> Date: Mon, 24 Apr 2023 11:52:02 -0400 Subject: [PATCH 07/21] reduce slide arg check test coverage We only need to make sure errors/warnings are raised in a subset of cases to make sure `f`s are being passed correctly to the check function. There are separate tests for the check function that are more exhaustive. --- tests/testthat/test-epi_slide.R | 17 ----------------- tests/testthat/test-epix_slide.R | 17 ----------------- 2 files changed, 34 deletions(-) diff --git a/tests/testthat/test-epi_slide.R b/tests/testthat/test-epi_slide.R index c09781f51..01b879083 100644 --- a/tests/testthat/test-epi_slide.R +++ b/tests/testthat/test-epi_slide.R @@ -89,29 +89,12 @@ test_that("these doesn't produce an error; the error appears only if the ref tim test_that("epi_slide alerts if the provided f doesn't take enough args", { f_xg = function(x, g) dplyr::tibble(value=mean(x$value), count=length(x$value)) - f_xg_dots = function(x, g, ...) dplyr::tibble(value=mean(x$value), count=length(x$value)) - # If `regexp` is NA, asserts that there should be no errors/messages. expect_error(epi_slide(grouped, f_xg, before = 1L, ref_time_values = d+1), regexp = NA) expect_warning(epi_slide(grouped, f_xg, before = 1L, ref_time_values = d+1), regexp = NA) - expect_error(epi_slide(grouped, f_xg_dots, before = 1L, ref_time_values = d+1), regexp = NA) - expect_warning(epi_slide(grouped, f_xg_dots, before = 1L, ref_time_values = d+1), regexp = NA) f_x_dots = function(x, ...) dplyr::tibble(value=mean(x$value), count=length(x$value)) - f_dots = function(...) dplyr::tibble(value=c(5), count=c(2)) - f_x = function(x) dplyr::tibble(value=mean(x$value), count=length(x$value)) - f = function() dplyr::tibble(value=c(5), count=c(2)) - expect_warning(epi_slide(grouped, f_x_dots, before = 1L, ref_time_values = d+1), regexp = "positional arguments before the `...` args", class = "check_sufficient_f_args__f_needs_min_args_before_dots") - expect_warning(epi_slide(grouped, f_dots, before = 1L, ref_time_values = d+1), - regexp = "positional arguments before the `...` args", - class = "check_sufficient_f_args__f_needs_min_args_before_dots") - expect_error(epi_slide(grouped, f_x, before = 1L, ref_time_values = d+1), - regexp = "`f` must take at least", - class = "check_sufficient_f_args__f_needs_min_args") - expect_error(epi_slide(grouped, f, before = 1L, ref_time_values = d+1), - regexp = "`f` must take at least", - class = "check_sufficient_f_args__f_needs_min_args") }) diff --git a/tests/testthat/test-epix_slide.R b/tests/testthat/test-epix_slide.R index 587e5913d..7c4b1df66 100644 --- a/tests/testthat/test-epix_slide.R +++ b/tests/testthat/test-epix_slide.R @@ -351,29 +351,12 @@ test_that("epix_slide with all_versions option works as intended",{ test_that("epix_slide alerts if the provided f doesn't take enough args", { f_xg = function(x, g) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) - f_xg_dots = function(x, g, ...) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) - # If `regexp` is NA, asserts that there should be no errors/messages. expect_error(epix_slide(xx, f = f_xg, before = 2L), regexp = NA) expect_warning(epix_slide(xx, f = f_xg, before = 2L), regexp = NA) - expect_error(epix_slide(xx, f = f_xg_dots, before = 2L), regexp = NA) - expect_warning(epix_slide(xx, f = f_xg_dots, before = 2L), regexp = NA) f_x_dots = function(x, ...) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) - f_dots = function(...) dplyr::tibble(value=c(5), count=c(2)) - f_x = function(x) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) - f = function() dplyr::tibble(value=c(5), count=c(2)) - expect_warning(epix_slide(xx, f_x_dots, before = 2L), regexp = "positional arguments before the `...` args", class = "check_sufficient_f_args__f_needs_min_args_before_dots") - expect_warning(epix_slide(xx, f_dots, before = 2L), - regexp = "positional arguments before the `...` args", - class = "check_sufficient_f_args__f_needs_min_args_before_dots") - expect_error(epix_slide(xx, f_x, before = 2L), - regexp = "`f` must take at least", - class = "check_sufficient_f_args__f_needs_min_args") - expect_error(epix_slide(xx, f, before = 2L), - regexp = "`f` must take at least", - class = "check_sufficient_f_args__f_needs_min_args") }) From a3f227c932fff2d58453b1be3267990af6c427e6 Mon Sep 17 00:00:00 2001 From: "Logan C. Brooks" Date: Mon, 8 May 2023 15:35:20 -0700 Subject: [PATCH 08/21] Rename check->assert_sufficient_f_args, tweak warning text & fields --- R/grouped_epi_archive.R | 2 +- R/slide.R | 2 +- R/utils.R | 16 ++++++++-------- tests/testthat/test-epi_slide.R | 2 +- tests/testthat/test-epix_slide.R | 2 +- tests/testthat/test-utils.R | 26 +++++++++++++------------- 6 files changed, 25 insertions(+), 25 deletions(-) diff --git a/R/grouped_epi_archive.R b/R/grouped_epi_archive.R index ac093e6fc..06b5aa118 100644 --- a/R/grouped_epi_archive.R +++ b/R/grouped_epi_archive.R @@ -222,7 +222,7 @@ grouped_epi_archive = # Check that `f` takes enough args if (!missing(f) && is.function(f)) { - check_sufficient_f_args(f) + assert_sufficient_f_args(f) } # Validate and pre-process `before`: diff --git a/R/slide.R b/R/slide.R index 39f55089f..a8eb06f58 100644 --- a/R/slide.R +++ b/R/slide.R @@ -158,7 +158,7 @@ epi_slide = function(x, f, ..., before, after, ref_time_values, # Check that `f` takes enough args if (!missing(f) && is.function(f)) { - check_sufficient_f_args(f) + assert_sufficient_f_args(f) } # Arrange by increasing time_value diff --git a/R/utils.R b/R/utils.R index c953b385d..cac5b2b2a 100644 --- a/R/utils.R +++ b/R/utils.R @@ -100,30 +100,30 @@ paste_lines = function(lines) { Abort = function(msg, ...) rlang::abort(break_str(msg, init = "Error: "), ...) Warn = function(msg, ...) rlang::warn(break_str(msg, init = "Warning: "), ...) -#' Check that a sliding computation function takes enough args +#' Assert that a sliding computation function takes enough args #' #' @param f Function; specifies a computation to slide over an `epi_df` or #' `epi_archive` in `epi_slide` or `epix_slide`. #' #' @noRd -check_sufficient_f_args <- function(f) { +assert_sufficient_f_args <- function(f) { n_mandatory_f_args <- 2 arg_names = names(formals(args(f))) if ("..." %in% arg_names) { # Keep all arg names before `...` dots_i <- which(arg_names == "...") - arg_names <- arg_names[seq_len(dots_i - 1)] + arg_names_before_dots <- arg_names[seq_len(dots_i - 1)] - if (length(arg_names) < n_mandatory_f_args) { - Warn(sprintf("`f` only takes %s positional arguments before the `...` args, but %s were expected; this can lead to obtuse errors downstream", length(arg_names), n_mandatory_f_args), - class="check_sufficient_f_args__f_needs_min_args_before_dots", + if (length(arg_names_before_dots) < n_mandatory_f_args) { + Warn(sprintf("`f` only takes %s positional arguments before the `...` args, but `epi[x]_slide` will call it with at least %s positional arguments; if `f` doesn't expect those arguments, it may produce confusing error messages", length(arg_names), n_mandatory_f_args), + class="epiprocess__check_sufficient_f_args__f_needs_min_args_before_dots", epiprocess__f = f, - epiprocess__arg_names = arg_names) + epiprocess__arg_names_before_dots = arg_names_before_dots) } } else { if (length(arg_names) < n_mandatory_f_args) { Abort(sprintf("`f` must take at least %s arguments", n_mandatory_f_args), - class="check_sufficient_f_args__f_needs_min_args", + class="epiprocess__check_sufficient_f_args__f_needs_min_args", epiprocess__f = f, epiprocess__arg_names = arg_names) } diff --git a/tests/testthat/test-epi_slide.R b/tests/testthat/test-epi_slide.R index 01b879083..77a725bd7 100644 --- a/tests/testthat/test-epi_slide.R +++ b/tests/testthat/test-epi_slide.R @@ -96,5 +96,5 @@ test_that("epi_slide alerts if the provided f doesn't take enough args", { f_x_dots = function(x, ...) dplyr::tibble(value=mean(x$value), count=length(x$value)) expect_warning(epi_slide(grouped, f_x_dots, before = 1L, ref_time_values = d+1), regexp = "positional arguments before the `...` args", - class = "check_sufficient_f_args__f_needs_min_args_before_dots") + class = "epiprocess__check_sufficient_f_args__f_needs_min_args_before_dots") }) diff --git a/tests/testthat/test-epix_slide.R b/tests/testthat/test-epix_slide.R index 7c4b1df66..e6f3606fa 100644 --- a/tests/testthat/test-epix_slide.R +++ b/tests/testthat/test-epix_slide.R @@ -358,5 +358,5 @@ test_that("epix_slide alerts if the provided f doesn't take enough args", { f_x_dots = function(x, ...) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) expect_warning(epix_slide(xx, f_x_dots, before = 2L), regexp = "positional arguments before the `...` args", - class = "check_sufficient_f_args__f_needs_min_args_before_dots") + class = "epiprocess__check_sufficient_f_args__f_needs_min_args_before_dots") }) diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index 56113a02a..ea21ac710 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -109,31 +109,31 @@ test_that("enlist works",{ expect_true(inherits(my_list,"list")) }) -test_that("check_sufficient_f_args alerts if the provided f doesn't take enough args", { +test_that("assert_sufficient_f_args alerts if the provided f doesn't take enough args", { f_xg = function(x, g) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) f_xg_dots = function(x, g, ...) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) # If `regexp` is NA, asserts that there should be no errors/messages. - expect_error(check_sufficient_f_args(f_xg), regexp = NA) - expect_warning(check_sufficient_f_args(f_xg), regexp = NA) - expect_error(check_sufficient_f_args(f_xg_dots), regexp = NA) - expect_warning(check_sufficient_f_args(f_xg_dots), regexp = NA) + expect_error(assert_sufficient_f_args(f_xg), regexp = NA) + expect_warning(assert_sufficient_f_args(f_xg), regexp = NA) + expect_error(assert_sufficient_f_args(f_xg_dots), regexp = NA) + expect_warning(assert_sufficient_f_args(f_xg_dots), regexp = NA) f_x_dots = function(x, ...) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) f_dots = function(...) dplyr::tibble(value=c(5), count=c(2)) f_x = function(x) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) f = function() dplyr::tibble(value=c(5), count=c(2)) - expect_warning(check_sufficient_f_args(f_x_dots), + expect_warning(assert_sufficient_f_args(f_x_dots), regexp = "positional arguments before the `...` args", - class = "check_sufficient_f_args__f_needs_min_args_before_dots") - expect_warning(check_sufficient_f_args(f_dots), + class = "epiprocess__check_sufficient_f_args__f_needs_min_args_before_dots") + expect_warning(assert_sufficient_f_args(f_dots), regexp = "positional arguments before the `...` args", - class = "check_sufficient_f_args__f_needs_min_args_before_dots") - expect_error(check_sufficient_f_args(f_x), + class = "epiprocess__check_sufficient_f_args__f_needs_min_args_before_dots") + expect_error(assert_sufficient_f_args(f_x), regexp = "`f` must take at least", - class = "check_sufficient_f_args__f_needs_min_args") - expect_error(check_sufficient_f_args(f), + class = "epiprocess__check_sufficient_f_args__f_needs_min_args") + expect_error(assert_sufficient_f_args(f), regexp = "`f` must take at least", - class = "check_sufficient_f_args__f_needs_min_args") + class = "epiprocess__check_sufficient_f_args__f_needs_min_args") }) From 6825a5cc2813b05d117008283b86fc119ef24b0c Mon Sep 17 00:00:00 2001 From: Nat DeFries <42820733+nmdefries@users.noreply.github.com> Date: Wed, 10 May 2023 13:08:29 -0400 Subject: [PATCH 09/21] rename error class to match func name --- R/utils.R | 4 ++-- tests/testthat/test-epi_slide.R | 2 +- tests/testthat/test-epix_slide.R | 2 +- tests/testthat/test-utils.R | 8 ++++---- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/R/utils.R b/R/utils.R index cac5b2b2a..68f601778 100644 --- a/R/utils.R +++ b/R/utils.R @@ -116,14 +116,14 @@ assert_sufficient_f_args <- function(f) { if (length(arg_names_before_dots) < n_mandatory_f_args) { Warn(sprintf("`f` only takes %s positional arguments before the `...` args, but `epi[x]_slide` will call it with at least %s positional arguments; if `f` doesn't expect those arguments, it may produce confusing error messages", length(arg_names), n_mandatory_f_args), - class="epiprocess__check_sufficient_f_args__f_needs_min_args_before_dots", + class="epiprocess__assert_sufficient_f_args__f_needs_min_args_before_dots", epiprocess__f = f, epiprocess__arg_names_before_dots = arg_names_before_dots) } } else { if (length(arg_names) < n_mandatory_f_args) { Abort(sprintf("`f` must take at least %s arguments", n_mandatory_f_args), - class="epiprocess__check_sufficient_f_args__f_needs_min_args", + class="epiprocess__assert_sufficient_f_args__f_needs_min_args", epiprocess__f = f, epiprocess__arg_names = arg_names) } diff --git a/tests/testthat/test-epi_slide.R b/tests/testthat/test-epi_slide.R index 77a725bd7..1075d23e2 100644 --- a/tests/testthat/test-epi_slide.R +++ b/tests/testthat/test-epi_slide.R @@ -96,5 +96,5 @@ test_that("epi_slide alerts if the provided f doesn't take enough args", { f_x_dots = function(x, ...) dplyr::tibble(value=mean(x$value), count=length(x$value)) expect_warning(epi_slide(grouped, f_x_dots, before = 1L, ref_time_values = d+1), regexp = "positional arguments before the `...` args", - class = "epiprocess__check_sufficient_f_args__f_needs_min_args_before_dots") + class = "epiprocess__assert_sufficient_f_args__f_needs_min_args_before_dots") }) diff --git a/tests/testthat/test-epix_slide.R b/tests/testthat/test-epix_slide.R index e6f3606fa..4f0d05f92 100644 --- a/tests/testthat/test-epix_slide.R +++ b/tests/testthat/test-epix_slide.R @@ -358,5 +358,5 @@ test_that("epix_slide alerts if the provided f doesn't take enough args", { f_x_dots = function(x, ...) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) expect_warning(epix_slide(xx, f_x_dots, before = 2L), regexp = "positional arguments before the `...` args", - class = "epiprocess__check_sufficient_f_args__f_needs_min_args_before_dots") + class = "epiprocess__assert_sufficient_f_args__f_needs_min_args_before_dots") }) diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index ea21ac710..e90610883 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -126,14 +126,14 @@ test_that("assert_sufficient_f_args alerts if the provided f doesn't take enough expect_warning(assert_sufficient_f_args(f_x_dots), regexp = "positional arguments before the `...` args", - class = "epiprocess__check_sufficient_f_args__f_needs_min_args_before_dots") + class = "epiprocess__assert_sufficient_f_args__f_needs_min_args_before_dots") expect_warning(assert_sufficient_f_args(f_dots), regexp = "positional arguments before the `...` args", - class = "epiprocess__check_sufficient_f_args__f_needs_min_args_before_dots") + class = "epiprocess__assert_sufficient_f_args__f_needs_min_args_before_dots") expect_error(assert_sufficient_f_args(f_x), regexp = "`f` must take at least", - class = "epiprocess__check_sufficient_f_args__f_needs_min_args") + class = "epiprocess__assert_sufficient_f_args__f_needs_min_args") expect_error(assert_sufficient_f_args(f), regexp = "`f` must take at least", - class = "epiprocess__check_sufficient_f_args__f_needs_min_args") + class = "epiprocess__assert_sufficient_f_args__f_needs_min_args") }) From 51b12ee44b3c1415bec190c326e6eb62efbdd463 Mon Sep 17 00:00:00 2001 From: Nat DeFries <42820733+nmdefries@users.noreply.github.com> Date: Wed, 10 May 2023 13:10:40 -0400 Subject: [PATCH 10/21] drop regexp in tests where also specify error class --- tests/testthat/test-epi_slide.R | 1 - tests/testthat/test-epix_slide.R | 1 - tests/testthat/test-utils.R | 4 ---- 3 files changed, 6 deletions(-) diff --git a/tests/testthat/test-epi_slide.R b/tests/testthat/test-epi_slide.R index 1075d23e2..5c252ab1d 100644 --- a/tests/testthat/test-epi_slide.R +++ b/tests/testthat/test-epi_slide.R @@ -95,6 +95,5 @@ test_that("epi_slide alerts if the provided f doesn't take enough args", { f_x_dots = function(x, ...) dplyr::tibble(value=mean(x$value), count=length(x$value)) expect_warning(epi_slide(grouped, f_x_dots, before = 1L, ref_time_values = d+1), - regexp = "positional arguments before the `...` args", class = "epiprocess__assert_sufficient_f_args__f_needs_min_args_before_dots") }) diff --git a/tests/testthat/test-epix_slide.R b/tests/testthat/test-epix_slide.R index 4f0d05f92..e7c7210c3 100644 --- a/tests/testthat/test-epix_slide.R +++ b/tests/testthat/test-epix_slide.R @@ -357,6 +357,5 @@ test_that("epix_slide alerts if the provided f doesn't take enough args", { f_x_dots = function(x, ...) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) expect_warning(epix_slide(xx, f_x_dots, before = 2L), - regexp = "positional arguments before the `...` args", class = "epiprocess__assert_sufficient_f_args__f_needs_min_args_before_dots") }) diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index e90610883..eb8873171 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -125,15 +125,11 @@ test_that("assert_sufficient_f_args alerts if the provided f doesn't take enough f = function() dplyr::tibble(value=c(5), count=c(2)) expect_warning(assert_sufficient_f_args(f_x_dots), - regexp = "positional arguments before the `...` args", class = "epiprocess__assert_sufficient_f_args__f_needs_min_args_before_dots") expect_warning(assert_sufficient_f_args(f_dots), - regexp = "positional arguments before the `...` args", class = "epiprocess__assert_sufficient_f_args__f_needs_min_args_before_dots") expect_error(assert_sufficient_f_args(f_x), - regexp = "`f` must take at least", class = "epiprocess__assert_sufficient_f_args__f_needs_min_args") expect_error(assert_sufficient_f_args(f), - regexp = "`f` must take at least", class = "epiprocess__assert_sufficient_f_args__f_needs_min_args") }) From d5466d38db3a149fe2a990e6f91b1561836abab0 Mon Sep 17 00:00:00 2001 From: Nat DeFries <42820733+nmdefries@users.noreply.github.com> Date: Thu, 11 May 2023 14:45:42 -0400 Subject: [PATCH 11/21] check if required fields already have defaults set --- NAMESPACE | 2 ++ R/utils.R | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/NAMESPACE b/NAMESPACE index 10847e6c9..db2d95d4b 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -84,6 +84,7 @@ importFrom(dplyr,ungroup) importFrom(lubridate,days) importFrom(lubridate,weeks) importFrom(magrittr,"%>%") +importFrom(purrr,map_lgl) importFrom(rlang,"!!!") importFrom(rlang,"!!") importFrom(rlang,.data) @@ -91,6 +92,7 @@ importFrom(rlang,.env) importFrom(rlang,arg_match) importFrom(rlang,enquo) importFrom(rlang,enquos) +importFrom(rlang,is_missing) importFrom(rlang,is_quosure) importFrom(rlang,quo_is_missing) importFrom(rlang,sym) diff --git a/R/utils.R b/R/utils.R index 68f601778..395f69f7c 100644 --- a/R/utils.R +++ b/R/utils.R @@ -105,10 +105,14 @@ Warn = function(msg, ...) rlang::warn(break_str(msg, init = "Warning: "), ...) #' @param f Function; specifies a computation to slide over an `epi_df` or #' `epi_archive` in `epi_slide` or `epix_slide`. #' +#' @importFrom rlang is_missing +#' @importFrom purrr map_lgl +#' #' @noRd assert_sufficient_f_args <- function(f) { n_mandatory_f_args <- 2 - arg_names = names(formals(args(f))) + args_f = formals(args(f)) + arg_names = names(args_f) if ("..." %in% arg_names) { # Keep all arg names before `...` dots_i <- which(arg_names == "...") @@ -128,6 +132,18 @@ assert_sufficient_f_args <- function(f) { epiprocess__arg_names = arg_names) } } + # If `f` has fewer than n_mandatory_f_args before `...`, then we only need + # to check those args for defaults. + n_args_before_dots = min( + ifelse(exists("arg_names_before_dots"), length(arg_names_before_dots), n_mandatory_f_args), + n_mandatory_f_args + ) + if ( any(map_lgl(args_f[seq(n_args_before_dots)], ~!is_missing(.x))) ) { + Abort(sprintf("Some of `f`'s first %s arguments use defaults that would be overridden when calling `f` to run the computation", n_args_before_dots), + class="epiprocess__assert_sufficient_f_args__required_args_contain_defaults", + epiprocess__f = f, + epiprocess__args_f = args_f) + } } ########## From 46cf783ec3896c2dee73f9df1611889b80f22c4d Mon Sep 17 00:00:00 2001 From: Nat DeFries <42820733+nmdefries@users.noreply.github.com> Date: Thu, 11 May 2023 14:56:25 -0400 Subject: [PATCH 12/21] test default checking --- tests/testthat/test-utils.R | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index eb8873171..c3fb772ae 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -133,3 +133,16 @@ test_that("assert_sufficient_f_args alerts if the provided f doesn't take enough expect_error(assert_sufficient_f_args(f), class = "epiprocess__assert_sufficient_f_args__f_needs_min_args") }) + +test_that("assert_sufficient_f_args alerts if the provided f has defaults for the required args", { + f_xg = function(x, g=1) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) + f_xg_dots = function(x=1, g, ...) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) + f_x_dots = function(x=1, ...) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) + + expect_error(assert_sufficient_f_args(f_xg), + class = "epiprocess__assert_sufficient_f_args__required_args_contain_defaults") + expect_error(assert_sufficient_f_args(f_xg_dots), + class = "epiprocess__assert_sufficient_f_args__required_args_contain_defaults") + expect_error(assert_sufficient_f_args(f_x_dots), + class = "epiprocess__assert_sufficient_f_args__required_args_contain_defaults") +}) From a572464f18109e03850be0691dfc453bd04b130a Mon Sep 17 00:00:00 2001 From: "Logan C. Brooks" Date: Mon, 15 May 2023 16:44:09 -0700 Subject: [PATCH 13/21] Suppress forwarded warning from warning+error sufficient-args test --- tests/testthat/test-utils.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index c3fb772ae..4536e0cda 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -143,6 +143,6 @@ test_that("assert_sufficient_f_args alerts if the provided f has defaults for th class = "epiprocess__assert_sufficient_f_args__required_args_contain_defaults") expect_error(assert_sufficient_f_args(f_xg_dots), class = "epiprocess__assert_sufficient_f_args__required_args_contain_defaults") - expect_error(assert_sufficient_f_args(f_x_dots), + expect_error(suppressWarnings(assert_sufficient_f_args(f_x_dots)), class = "epiprocess__assert_sufficient_f_args__required_args_contain_defaults") }) From b982526da1abbb7e2ada6dbf52524403f21fecec Mon Sep 17 00:00:00 2001 From: "Logan C. Brooks" Date: Mon, 15 May 2023 19:06:10 -0700 Subject: [PATCH 14/21] Account for `...` forwarding in `assert_sufficient_f_args` --- DESCRIPTION | 5 ++- R/grouped_epi_archive.R | 2 +- R/slide.R | 2 +- R/utils.R | 76 ++++++++++++++++++++------------ tests/testthat/test-epi_slide.R | 2 +- tests/testthat/test-epix_slide.R | 2 +- tests/testthat/test-utils.R | 28 +++++++++++- 7 files changed, 80 insertions(+), 37 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 8a06e6f56..6fecc73c4 100755 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -21,7 +21,8 @@ Description: This package introduces a common data structure for epidemiological work with revisions to these data sets over time, and offers associated utilities to perform basic signal processing tasks. License: MIT + file LICENSE -Imports: +Imports: + cli, data.table, dplyr (>= 1.0.0), fabletools, @@ -48,7 +49,7 @@ Suggests: knitr, outbreaks, rmarkdown, - testthat (>= 3.0.0), + testthat (>= 3.1.5), waldo (>= 0.3.1), withr VignetteBuilder: diff --git a/R/grouped_epi_archive.R b/R/grouped_epi_archive.R index 06b5aa118..76b079a4a 100644 --- a/R/grouped_epi_archive.R +++ b/R/grouped_epi_archive.R @@ -222,7 +222,7 @@ grouped_epi_archive = # Check that `f` takes enough args if (!missing(f) && is.function(f)) { - assert_sufficient_f_args(f) + assert_sufficient_f_args(f, ...) } # Validate and pre-process `before`: diff --git a/R/slide.R b/R/slide.R index a8eb06f58..d8d6becba 100644 --- a/R/slide.R +++ b/R/slide.R @@ -158,7 +158,7 @@ epi_slide = function(x, f, ..., before, after, ref_time_values, # Check that `f` takes enough args if (!missing(f) && is.function(f)) { - assert_sufficient_f_args(f) + assert_sufficient_f_args(f, ...) } # Arrange by increasing time_value diff --git a/R/utils.R b/R/utils.R index 395f69f7c..8b1b078c0 100644 --- a/R/utils.R +++ b/R/utils.R @@ -103,46 +103,64 @@ Warn = function(msg, ...) rlang::warn(break_str(msg, init = "Warning: "), ...) #' Assert that a sliding computation function takes enough args #' #' @param f Function; specifies a computation to slide over an `epi_df` or -#' `epi_archive` in `epi_slide` or `epix_slide`. +#' `epi_archive` in `epi_slide` or `epix_slide`. +#' @param ... Dots that will be forwarded to `f` from the dots of `epi_slide` or +#' `epix_slide`. #' #' @importFrom rlang is_missing #' @importFrom purrr map_lgl #' #' @noRd -assert_sufficient_f_args <- function(f) { - n_mandatory_f_args <- 2 - args_f = formals(args(f)) - arg_names = names(args_f) - if ("..." %in% arg_names) { +assert_sufficient_f_args <- function(f, ...) { + mandatory_f_args_labels <- c("window data", "group key") + n_mandatory_f_args <- length(mandatory_f_args_labels) + args = formals(args(f)) + args_names = names(args) + # Remove named arguments forwarded from `epi[x]_slide`'s `...`: + forwarded_dots_names = names(rlang::call_match(dots_expand = FALSE)[["..."]]) + args_matched_in_dots = + # positional calling args will skip over args matched by named calling args + args_names %in% forwarded_dots_names & + # extreme edge case: `epi[x]_slide(, dot = 1, `...` = 2)` + args_names != "..." + remaining_args = args[!args_matched_in_dots] + remaining_args_names = names(remaining_args) + dots_i <- which(remaining_args_names == "...") # integer(0) if no match + if (length(dots_i) != 0L) { # Keep all arg names before `...` - dots_i <- which(arg_names == "...") - arg_names_before_dots <- arg_names[seq_len(dots_i - 1)] - - if (length(arg_names_before_dots) < n_mandatory_f_args) { - Warn(sprintf("`f` only takes %s positional arguments before the `...` args, but `epi[x]_slide` will call it with at least %s positional arguments; if `f` doesn't expect those arguments, it may produce confusing error messages", length(arg_names), n_mandatory_f_args), - class="epiprocess__assert_sufficient_f_args__f_needs_min_args_before_dots", - epiprocess__f = f, - epiprocess__arg_names_before_dots = arg_names_before_dots) + mandatory_args_mapped_names <- remaining_args_names[seq_len(dots_i - 1L)] + + if (dots_i - 1L < n_mandatory_f_args) { + mandatory_f_args_in_f_dots = tail(mandatory_f_args_labels, dots_i - 1L) + Warn(sprintf("`f` might not have enough positional arguments before its `...`; in the current `epi[x]_slide` call, the %s will be included in `f`'s `...`; if `f` doesn't expect those arguments, it may produce confusing error messages", cli::ansi_collapse(mandatory_f_args_in_f_dots)), + class = "epiprocess__assert_sufficient_f_args__mandatory_f_args_passed_to_f_dots", + epiprocess__f = f, + epiprocess__mandatory_f_args_in_f_dots = mandatory_f_args_in_f_dots) } } else { - if (length(arg_names) < n_mandatory_f_args) { + if (length(remaining_args_names) < n_mandatory_f_args) { Abort(sprintf("`f` must take at least %s arguments", n_mandatory_f_args), - class="epiprocess__assert_sufficient_f_args__f_needs_min_args", - epiprocess__f = f, - epiprocess__arg_names = arg_names) + class = "epiprocess__assert_sufficient_f_args__f_needs_min_args", + epiprocess__f = f, + epiprocess__remaining_args_names = remaining_args_names) } } - # If `f` has fewer than n_mandatory_f_args before `...`, then we only need - # to check those args for defaults. - n_args_before_dots = min( - ifelse(exists("arg_names_before_dots"), length(arg_names_before_dots), n_mandatory_f_args), - n_mandatory_f_args - ) - if ( any(map_lgl(args_f[seq(n_args_before_dots)], ~!is_missing(.x))) ) { - Abort(sprintf("Some of `f`'s first %s arguments use defaults that would be overridden when calling `f` to run the computation", n_args_before_dots), - class="epiprocess__assert_sufficient_f_args__required_args_contain_defaults", - epiprocess__f = f, - epiprocess__args_f = args_f) + # Check for args with defaults that are filled with mandatory positional + # calling args. If `f` has fewer than n_mandatory_f_args before `...`, then we + # only need to check those args for defaults. Note that `dots_i - 1L` is + # length 0 if `f` doesn't accept `...`. + n_remaining_args_for_default_check = min(c(dots_i - 1L, n_mandatory_f_args)) + default_check_args = remaining_args[seq_len(n_remaining_args_for_default_check)] + default_check_args_names = names(default_check_args) + has_default_replaced_by_mandatory = map_lgl(default_check_args, ~!is_missing(.x)) + if (any(has_default_replaced_by_mandatory)) { + mandatory_args_replacing_defaults = + mandatory_f_args_labels[seq_len(sum(has_default_replaced_by_mandatory))] + args_with_default_replaced_by_mandatory = + rlang::syms(default_check_args_names[has_default_replaced_by_mandatory]) + cli::cli_abort("`epi[x]_slide` would pass the {mandatory_args_replacing_defaults} to `f`'s {args_with_default_replaced_by_mandatory} argument{?s}, which have default values; we suspect that `f` doesn't expect these args at all and may produce confusing error messages. Please add additional arguments to `f` or remove defaults as appropriate.", + class = "epiprocess__assert_sufficient_f_args__required_args_contain_defaults", + epiprocess__f = f) } } diff --git a/tests/testthat/test-epi_slide.R b/tests/testthat/test-epi_slide.R index 5c252ab1d..84192f940 100644 --- a/tests/testthat/test-epi_slide.R +++ b/tests/testthat/test-epi_slide.R @@ -95,5 +95,5 @@ test_that("epi_slide alerts if the provided f doesn't take enough args", { f_x_dots = function(x, ...) dplyr::tibble(value=mean(x$value), count=length(x$value)) expect_warning(epi_slide(grouped, f_x_dots, before = 1L, ref_time_values = d+1), - class = "epiprocess__assert_sufficient_f_args__f_needs_min_args_before_dots") + class = "epiprocess__assert_sufficient_f_args__mandatory_f_args_passed_to_f_dots") }) diff --git a/tests/testthat/test-epix_slide.R b/tests/testthat/test-epix_slide.R index e7c7210c3..5eeb5c2c1 100644 --- a/tests/testthat/test-epix_slide.R +++ b/tests/testthat/test-epix_slide.R @@ -357,5 +357,5 @@ test_that("epix_slide alerts if the provided f doesn't take enough args", { f_x_dots = function(x, ...) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) expect_warning(epix_slide(xx, f_x_dots, before = 2L), - class = "epiprocess__assert_sufficient_f_args__f_needs_min_args_before_dots") + class = "epiprocess__assert_sufficient_f_args__mandatory_f_args_passed_to_f_dots") }) diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index 4536e0cda..4f05b4247 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -125,9 +125,9 @@ test_that("assert_sufficient_f_args alerts if the provided f doesn't take enough f = function() dplyr::tibble(value=c(5), count=c(2)) expect_warning(assert_sufficient_f_args(f_x_dots), - class = "epiprocess__assert_sufficient_f_args__f_needs_min_args_before_dots") + class = "epiprocess__assert_sufficient_f_args__mandatory_f_args_passed_to_f_dots") expect_warning(assert_sufficient_f_args(f_dots), - class = "epiprocess__assert_sufficient_f_args__f_needs_min_args_before_dots") + class = "epiprocess__assert_sufficient_f_args__mandatory_f_args_passed_to_f_dots") expect_error(assert_sufficient_f_args(f_x), class = "epiprocess__assert_sufficient_f_args__f_needs_min_args") expect_error(assert_sufficient_f_args(f), @@ -145,4 +145,28 @@ test_that("assert_sufficient_f_args alerts if the provided f has defaults for th class = "epiprocess__assert_sufficient_f_args__required_args_contain_defaults") expect_error(suppressWarnings(assert_sufficient_f_args(f_x_dots)), class = "epiprocess__assert_sufficient_f_args__required_args_contain_defaults") + + f_xsg = function(x, setting="a", g) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) + f_xsg_dots = function(x, setting="a", g, ...) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) + f_xs_dots = function(x=1, setting="a", ...) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) + + # forwarding named dots should prevent some complaints: + expect_no_error(assert_sufficient_f_args(f_xsg, setting = "b")) + expect_no_error(assert_sufficient_f_args(f_xsg_dots, setting = "b")) + expect_error(suppressWarnings(assert_sufficient_f_args(f_xs_dots, setting = "b")), + regexp = "window data to `f`'s x argument", + class = "epiprocess__assert_sufficient_f_args__required_args_contain_defaults") + + # forwarding unnamed dots should not: + expect_error(assert_sufficient_f_args(f_xsg, "b"), + class = "epiprocess__assert_sufficient_f_args__required_args_contain_defaults") + expect_error(assert_sufficient_f_args(f_xsg_dots, "b"), + class = "epiprocess__assert_sufficient_f_args__required_args_contain_defaults") + expect_error(assert_sufficient_f_args(f_xs_dots, "b"), + class = "epiprocess__assert_sufficient_f_args__required_args_contain_defaults") + + # forwarding no dots should produce a different error message in some cases: + expect_error(assert_sufficient_f_args(f_xs_dots), + regexp = "window data and group key to `f`'s x and setting argument", + class = "epiprocess__assert_sufficient_f_args__required_args_contain_defaults") }) From 290b4b9057e6f26965f078cec9d30e62192ba0ef Mon Sep 17 00:00:00 2001 From: "Logan C. Brooks" Date: Mon, 15 May 2023 19:35:36 -0700 Subject: [PATCH 15/21] Consider unnamed dots forwarding in `assert_sufficient_f_args` --- R/utils.R | 20 ++++++++++++++------ tests/testthat/test-utils.R | 10 ++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/R/utils.R b/R/utils.R index 8b1b078c0..d43b7a70c 100644 --- a/R/utils.R +++ b/R/utils.R @@ -125,6 +125,7 @@ assert_sufficient_f_args <- function(f, ...) { args_names != "..." remaining_args = args[!args_matched_in_dots] remaining_args_names = names(remaining_args) + # note that this doesn't include unnamed args forwarded through `...`. dots_i <- which(remaining_args_names == "...") # integer(0) if no match if (length(dots_i) != 0L) { # Keep all arg names before `...` @@ -138,11 +139,18 @@ assert_sufficient_f_args <- function(f, ...) { epiprocess__mandatory_f_args_in_f_dots = mandatory_f_args_in_f_dots) } } else { - if (length(remaining_args_names) < n_mandatory_f_args) { - Abort(sprintf("`f` must take at least %s arguments", n_mandatory_f_args), - class = "epiprocess__assert_sufficient_f_args__f_needs_min_args", - epiprocess__f = f, - epiprocess__remaining_args_names = remaining_args_names) + if (length(args_names) < n_mandatory_f_args + rlang::dots_n(...)) { + if (rlang::dots_n(...) == 0L) { + # common case; try for friendlier error message + Abort(sprintf("`f` must take at least %s arguments", n_mandatory_f_args), + class = "epiprocess__assert_sufficient_f_args__f_needs_min_args", + epiprocess__f = f) + } else { + # less common; highlight that they are (accidentally?) using dots forwarding + Abort(sprintf("`f` must take at least %s arguments plus the %s arguments forwarded through `epi[x]_slide`'s `...`", n_mandatory_f_args, rlang::dots_n(...)), + class = "epiprocess__assert_sufficient_f_args__f_needs_min_args_plus_forwarded", + epiprocess__f = f) + } } } # Check for args with defaults that are filled with mandatory positional @@ -158,7 +166,7 @@ assert_sufficient_f_args <- function(f, ...) { mandatory_f_args_labels[seq_len(sum(has_default_replaced_by_mandatory))] args_with_default_replaced_by_mandatory = rlang::syms(default_check_args_names[has_default_replaced_by_mandatory]) - cli::cli_abort("`epi[x]_slide` would pass the {mandatory_args_replacing_defaults} to `f`'s {args_with_default_replaced_by_mandatory} argument{?s}, which have default values; we suspect that `f` doesn't expect these args at all and may produce confusing error messages. Please add additional arguments to `f` or remove defaults as appropriate.", + cli::cli_abort("`epi[x]_slide` would pass the {mandatory_args_replacing_defaults} to `f`'s {args_with_default_replaced_by_mandatory} argument{?s}, which {?has a/have} default value{?s}; we suspect that `f` doesn't expect {?this arg/these args} at all and may produce confusing error messages. Please add additional arguments to `f` or remove defaults as appropriate.", class = "epiprocess__assert_sufficient_f_args__required_args_contain_defaults", epiprocess__f = f) } diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index 4f05b4247..61c3d28ac 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -132,6 +132,16 @@ test_that("assert_sufficient_f_args alerts if the provided f doesn't take enough class = "epiprocess__assert_sufficient_f_args__f_needs_min_args") expect_error(assert_sufficient_f_args(f), class = "epiprocess__assert_sufficient_f_args__f_needs_min_args") + + f_xs_dots = function(x, setting="a", ...) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) + f_xs = function(x, setting="a") dplyr::tibble(value=mean(x$binary), count=length(x$binary)) + expect_warning(assert_sufficient_f_args(f_xs_dots, setting="b"), + class = "epiprocess__assert_sufficient_f_args__mandatory_f_args_passed_to_f_dots") + expect_error(assert_sufficient_f_args(f_xs, setting="b"), + class = "epiprocess__assert_sufficient_f_args__f_needs_min_args_plus_forwarded") + + expect_error(assert_sufficient_f_args(f_xg, "b"), + class = "epiprocess__assert_sufficient_f_args__f_needs_min_args_plus_forwarded") }) test_that("assert_sufficient_f_args alerts if the provided f has defaults for the required args", { From 34649f8e1d9a824fb80385851fe1634ded399fa6 Mon Sep 17 00:00:00 2001 From: Nat DeFries <42820733+nmdefries@users.noreply.github.com> Date: Wed, 17 May 2023 11:58:06 -0400 Subject: [PATCH 16/21] factor out "dots_i -1" to var --- R/utils.R | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/R/utils.R b/R/utils.R index d43b7a70c..350124971 100644 --- a/R/utils.R +++ b/R/utils.R @@ -127,12 +127,13 @@ assert_sufficient_f_args <- function(f, ...) { remaining_args_names = names(remaining_args) # note that this doesn't include unnamed args forwarded through `...`. dots_i <- which(remaining_args_names == "...") # integer(0) if no match + n_f_args_before_dots <- dots_i - 1L if (length(dots_i) != 0L) { # Keep all arg names before `...` - mandatory_args_mapped_names <- remaining_args_names[seq_len(dots_i - 1L)] + mandatory_args_mapped_names <- remaining_args_names[seq_len(n_f_args_before_dots)] - if (dots_i - 1L < n_mandatory_f_args) { - mandatory_f_args_in_f_dots = tail(mandatory_f_args_labels, dots_i - 1L) + if (n_f_args_before_dots < n_mandatory_f_args) { + mandatory_f_args_in_f_dots = tail(mandatory_f_args_labels, n_f_args_before_dots) Warn(sprintf("`f` might not have enough positional arguments before its `...`; in the current `epi[x]_slide` call, the %s will be included in `f`'s `...`; if `f` doesn't expect those arguments, it may produce confusing error messages", cli::ansi_collapse(mandatory_f_args_in_f_dots)), class = "epiprocess__assert_sufficient_f_args__mandatory_f_args_passed_to_f_dots", epiprocess__f = f, @@ -155,9 +156,9 @@ assert_sufficient_f_args <- function(f, ...) { } # Check for args with defaults that are filled with mandatory positional # calling args. If `f` has fewer than n_mandatory_f_args before `...`, then we - # only need to check those args for defaults. Note that `dots_i - 1L` is + # only need to check those args for defaults. Note that `n_f_args_before_dots` is # length 0 if `f` doesn't accept `...`. - n_remaining_args_for_default_check = min(c(dots_i - 1L, n_mandatory_f_args)) + n_remaining_args_for_default_check = min(c(n_f_args_before_dots, n_mandatory_f_args)) default_check_args = remaining_args[seq_len(n_remaining_args_for_default_check)] default_check_args_names = names(default_check_args) has_default_replaced_by_mandatory = map_lgl(default_check_args, ~!is_missing(.x)) From 6ebb1a5d3cbab7e0547e5c12bfd1d7a0e0109d55 Mon Sep 17 00:00:00 2001 From: Nat DeFries <42820733+nmdefries@users.noreply.github.com> Date: Wed, 17 May 2023 12:01:16 -0400 Subject: [PATCH 17/21] import tail --- NAMESPACE | 1 + R/utils.R | 1 + 2 files changed, 2 insertions(+) diff --git a/NAMESPACE b/NAMESPACE index db2d95d4b..065302d75 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -103,3 +103,4 @@ importFrom(tidyr,unnest) importFrom(tidyselect,eval_select) importFrom(tidyselect,starts_with) importFrom(tsibble,as_tsibble) +importFrom(utils,tail) diff --git a/R/utils.R b/R/utils.R index 350124971..e3c3eb98e 100644 --- a/R/utils.R +++ b/R/utils.R @@ -109,6 +109,7 @@ Warn = function(msg, ...) rlang::warn(break_str(msg, init = "Warning: "), ...) #' #' @importFrom rlang is_missing #' @importFrom purrr map_lgl +#' @importFrom utils tail #' #' @noRd assert_sufficient_f_args <- function(f, ...) { From f6836d3157efdb5be47122ded789c874ddc26f2a Mon Sep 17 00:00:00 2001 From: brookslogan Date: Thu, 18 May 2023 15:51:35 -0700 Subject: [PATCH 18/21] Message about right args when `f` default is suspiciously replaced Co-authored-by: nmdefries <42820733+nmdefries@users.noreply.github.com> --- R/utils.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/utils.R b/R/utils.R index e3c3eb98e..67b88fa29 100644 --- a/R/utils.R +++ b/R/utils.R @@ -165,7 +165,7 @@ assert_sufficient_f_args <- function(f, ...) { has_default_replaced_by_mandatory = map_lgl(default_check_args, ~!is_missing(.x)) if (any(has_default_replaced_by_mandatory)) { mandatory_args_replacing_defaults = - mandatory_f_args_labels[seq_len(sum(has_default_replaced_by_mandatory))] + mandatory_f_args_labels[has_default_replaced_by_mandatory] args_with_default_replaced_by_mandatory = rlang::syms(default_check_args_names[has_default_replaced_by_mandatory]) cli::cli_abort("`epi[x]_slide` would pass the {mandatory_args_replacing_defaults} to `f`'s {args_with_default_replaced_by_mandatory} argument{?s}, which {?has a/have} default value{?s}; we suspect that `f` doesn't expect {?this arg/these args} at all and may produce confusing error messages. Please add additional arguments to `f` or remove defaults as appropriate.", From 10fb9e4246cb122a6f22a482bb128558948d1589 Mon Sep 17 00:00:00 2001 From: "Logan C. Brooks" Date: Thu, 18 May 2023 15:52:15 -0700 Subject: [PATCH 19/21] Message about right args when they fall into `f`'s dots --- R/utils.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/utils.R b/R/utils.R index 67b88fa29..66d3a2062 100644 --- a/R/utils.R +++ b/R/utils.R @@ -134,7 +134,7 @@ assert_sufficient_f_args <- function(f, ...) { mandatory_args_mapped_names <- remaining_args_names[seq_len(n_f_args_before_dots)] if (n_f_args_before_dots < n_mandatory_f_args) { - mandatory_f_args_in_f_dots = tail(mandatory_f_args_labels, n_f_args_before_dots) + mandatory_f_args_in_f_dots = tail(mandatory_f_args_labels, -n_f_args_before_dots) Warn(sprintf("`f` might not have enough positional arguments before its `...`; in the current `epi[x]_slide` call, the %s will be included in `f`'s `...`; if `f` doesn't expect those arguments, it may produce confusing error messages", cli::ansi_collapse(mandatory_f_args_in_f_dots)), class = "epiprocess__assert_sufficient_f_args__mandatory_f_args_passed_to_f_dots", epiprocess__f = f, From f0f0105455adec90220daf3947146eaa19eaedfd Mon Sep 17 00:00:00 2001 From: "Logan C. Brooks" Date: Thu, 18 May 2023 16:01:14 -0700 Subject: [PATCH 20/21] Also message about args fed to `f` dots when it has dots first --- R/utils.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/R/utils.R b/R/utils.R index 66d3a2062..f9327f91a 100644 --- a/R/utils.R +++ b/R/utils.R @@ -134,7 +134,8 @@ assert_sufficient_f_args <- function(f, ...) { mandatory_args_mapped_names <- remaining_args_names[seq_len(n_f_args_before_dots)] if (n_f_args_before_dots < n_mandatory_f_args) { - mandatory_f_args_in_f_dots = tail(mandatory_f_args_labels, -n_f_args_before_dots) + mandatory_f_args_in_f_dots = + tail(mandatory_f_args_labels, n_mandatory_f_args - n_f_args_before_dots) Warn(sprintf("`f` might not have enough positional arguments before its `...`; in the current `epi[x]_slide` call, the %s will be included in `f`'s `...`; if `f` doesn't expect those arguments, it may produce confusing error messages", cli::ansi_collapse(mandatory_f_args_in_f_dots)), class = "epiprocess__assert_sufficient_f_args__mandatory_f_args_passed_to_f_dots", epiprocess__f = f, From a8727287470baf087bef688b7e565350d3eaf4a7 Mon Sep 17 00:00:00 2001 From: "Logan C. Brooks" Date: Thu, 18 May 2023 16:27:13 -0700 Subject: [PATCH 21/21] Fix and test some other corner cases in f arg checking * Add some explanatory comments * Use `cli_warn` instead of `sprintf` with `ansi_collapse` so length-2 things are formatted "thing 1 and thing 2" not "thing 1, and thing 2" * Fix default-replacement error message when some mandatory args are absorbed by `f`'s dots. (This could give a faulty message due to broadcasting when there was just one arg with a default before the dots, or potentially an R error in other situations if/when there are >2 mandatory args.) * Add regexp's to our tests to test some of this message preparation. --- R/utils.R | 22 ++++++++++++++-------- tests/testthat/test-utils.R | 4 ++++ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/R/utils.R b/R/utils.R index f9327f91a..d17f05d4e 100644 --- a/R/utils.R +++ b/R/utils.R @@ -129,20 +129,23 @@ assert_sufficient_f_args <- function(f, ...) { # note that this doesn't include unnamed args forwarded through `...`. dots_i <- which(remaining_args_names == "...") # integer(0) if no match n_f_args_before_dots <- dots_i - 1L - if (length(dots_i) != 0L) { + if (length(dots_i) != 0L) { # `f` has a dots "arg" # Keep all arg names before `...` mandatory_args_mapped_names <- remaining_args_names[seq_len(n_f_args_before_dots)] if (n_f_args_before_dots < n_mandatory_f_args) { mandatory_f_args_in_f_dots = tail(mandatory_f_args_labels, n_mandatory_f_args - n_f_args_before_dots) - Warn(sprintf("`f` might not have enough positional arguments before its `...`; in the current `epi[x]_slide` call, the %s will be included in `f`'s `...`; if `f` doesn't expect those arguments, it may produce confusing error messages", cli::ansi_collapse(mandatory_f_args_in_f_dots)), - class = "epiprocess__assert_sufficient_f_args__mandatory_f_args_passed_to_f_dots", - epiprocess__f = f, - epiprocess__mandatory_f_args_in_f_dots = mandatory_f_args_in_f_dots) + cli::cli_warn( + "`f` might not have enough positional arguments before its `...`; in the current `epi[x]_slide` call, the {mandatory_f_args_in_f_dots} will be included in `f`'s `...`; if `f` doesn't expect those arguments, it may produce confusing error messages", + class = "epiprocess__assert_sufficient_f_args__mandatory_f_args_passed_to_f_dots", + epiprocess__f = f, + epiprocess__mandatory_f_args_in_f_dots = mandatory_f_args_in_f_dots + ) } - } else { + } else { # `f` doesn't have a dots "arg" if (length(args_names) < n_mandatory_f_args + rlang::dots_n(...)) { + # `f` doesn't take enough args. if (rlang::dots_n(...) == 0L) { # common case; try for friendlier error message Abort(sprintf("`f` must take at least %s arguments", n_mandatory_f_args), @@ -150,7 +153,7 @@ assert_sufficient_f_args <- function(f, ...) { epiprocess__f = f) } else { # less common; highlight that they are (accidentally?) using dots forwarding - Abort(sprintf("`f` must take at least %s arguments plus the %s arguments forwarded through `epi[x]_slide`'s `...`", n_mandatory_f_args, rlang::dots_n(...)), + Abort(sprintf("`f` must take at least %s arguments plus the %s arguments forwarded through `epi[x]_slide`'s `...`, or a named argument to `epi[x]_slide` was misspelled", n_mandatory_f_args, rlang::dots_n(...)), class = "epiprocess__assert_sufficient_f_args__f_needs_min_args_plus_forwarded", epiprocess__f = f) } @@ -165,8 +168,11 @@ assert_sufficient_f_args <- function(f, ...) { default_check_args_names = names(default_check_args) has_default_replaced_by_mandatory = map_lgl(default_check_args, ~!is_missing(.x)) if (any(has_default_replaced_by_mandatory)) { + default_check_mandatory_args_labels = + mandatory_f_args_labels[seq_len(n_remaining_args_for_default_check)] + # ^ excludes any mandatory args absorbed by f's `...`'s: mandatory_args_replacing_defaults = - mandatory_f_args_labels[has_default_replaced_by_mandatory] + default_check_mandatory_args_labels[has_default_replaced_by_mandatory] args_with_default_replaced_by_mandatory = rlang::syms(default_check_args_names[has_default_replaced_by_mandatory]) cli::cli_abort("`epi[x]_slide` would pass the {mandatory_args_replacing_defaults} to `f`'s {args_with_default_replaced_by_mandatory} argument{?s}, which {?has a/have} default value{?s}; we suspect that `f` doesn't expect {?this arg/these args} at all and may produce confusing error messages. Please add additional arguments to `f` or remove defaults as appropriate.", diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index 61c3d28ac..6648ce3ce 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -125,8 +125,10 @@ test_that("assert_sufficient_f_args alerts if the provided f doesn't take enough f = function() dplyr::tibble(value=c(5), count=c(2)) expect_warning(assert_sufficient_f_args(f_x_dots), + regexp = ", the group key will be included", class = "epiprocess__assert_sufficient_f_args__mandatory_f_args_passed_to_f_dots") expect_warning(assert_sufficient_f_args(f_dots), + regexp = ", the window data and group key will be included", class = "epiprocess__assert_sufficient_f_args__mandatory_f_args_passed_to_f_dots") expect_error(assert_sufficient_f_args(f_x), class = "epiprocess__assert_sufficient_f_args__f_needs_min_args") @@ -150,8 +152,10 @@ test_that("assert_sufficient_f_args alerts if the provided f has defaults for th f_x_dots = function(x=1, ...) dplyr::tibble(value=mean(x$binary), count=length(x$binary)) expect_error(assert_sufficient_f_args(f_xg), + regexp = "pass the group key to `f`'s g argument,", class = "epiprocess__assert_sufficient_f_args__required_args_contain_defaults") expect_error(assert_sufficient_f_args(f_xg_dots), + regexp = "pass the window data to `f`'s x argument,", class = "epiprocess__assert_sufficient_f_args__required_args_contain_defaults") expect_error(suppressWarnings(assert_sufficient_f_args(f_x_dots)), class = "epiprocess__assert_sufficient_f_args__required_args_contain_defaults")