Merge pull request #533 from cmu-delphi/lcb/hint-on-nontidy-f-force-errors

feat(epi[x]_slide): hint on forgotten syntax specifying comp

Author: brookslogan

DESCRIPTION

@@ -1,7 +1,7 @@
 Type: Package
 Package: epiprocess
 Title: Tools for basic signal processing in epidemiology
-Version: 0.9.2
+Version: 0.9.3
 Authors@R: c(
     person("Jacob", "Bien", role = "ctb"),
     person("Logan", "Brooks", , "[email protected]", role = c("aut", "cre")),

NEWS.md

@@ -12,6 +12,9 @@ Pre-1.0.0 numbering scheme: 0.x will indicate releases, while 0.x.y will indicat
 
 ## Improvements
 
+- `epi_slide` and `epix_slide` now provide some hints if you forget a `~` when
+  using a formula to specify the slide computation, and other bits of forgotten
+  syntax.
 - Improved validation of `.window_size` arguments.
 
 # epiprocess 0.9

R/grouped_epi_archive.R

@@ -312,14 +312,15 @@ epix_slide.grouped_epi_archive <- function(
       cli_abort("If `f` is missing then a computation must be specified via `...`.")
     }
 
-    .slide_comp <- as_diagonal_slide_computation(quosures)
+    .f_arg <- ".f" # dummy val, shouldn't be used since we're not using `.f`
+    .slide_comp <- as_diagonal_slide_computation(quosures, .f_arg = .f_arg)
     # Magic value that passes zero args as dots in calls below. Equivalent to
     # `... <- missing_arg()`, but use `assign` to avoid warning about
     # improper use of dots.
     assign("...", missing_arg())
   } else {
     used_data_masking <- FALSE
-    .slide_comp <- as_diagonal_slide_computation(.f, ...)
+    .slide_comp <- as_diagonal_slide_computation(.f, ..., .f_arg = caller_arg(.f))
   }
 
   # Computation for one group, one time value

R/slide.R

@@ -147,14 +147,16 @@ epi_slide <- function(
     }
 
     .f <- quosures
+    .f_arg <- ".f" # dummy val, shouldn't be used since we're not using `.f`
     # Magic value that passes zero args as dots in calls below. Equivalent to
     # `... <- missing_arg()`, but `assign` avoids warning about improper use of
     # dots.
     assign("...", missing_arg())
   } else {
     used_data_masking <- FALSE
+    .f_arg <- caller_arg(.f)
   }
-  .slide_comp <- as_time_slide_computation(.f, ...)
+  .slide_comp <- as_time_slide_computation(.f, ..., .f_arg = .f_arg)
 
   .align <- rlang::arg_match(.align)
   time_type <- attr(.x, "metadata")$time_type

R/utils.R

@@ -358,9 +358,48 @@ assert_sufficient_f_args <- function(.f, ..., .ref_time_value_label) {
 #' @importFrom rlang is_function new_function f_env is_environment missing_arg
 #'  f_rhs is_formula caller_arg caller_env
 #' @keywords internal
-as_slide_computation <- function(.f, ..., .ref_time_value_long_varnames, .ref_time_value_label) {
-  arg <- caller_arg(.f)
-  call <- caller_env()
+as_slide_computation <- function(.f, ...,
+                                 .f_arg = caller_arg(.f), .call = caller_env(),
+                                 .ref_time_value_long_varnames, .ref_time_value_label) {
+  if (".col_names" %in% rlang::call_args_names(rlang::call_match())) {
+    cli_abort(
+      c("{.code epi_slide} and {.code epix_slide} do not support `.col_names`;
+         consider:",
+        "*" = "using {.code epi_slide_mean}, {.code epi_slide_sum}, or
+               {.code epi_slide_opt}, if applicable",
+        "*" = "using {.code .f = ~ .x %>%
+               dplyr::reframe(across(your_col_names, list(your_func_name = your_func)))}"
+      ),
+      call = .call,
+      class = "epiprocess__as_slide_computation__given_.col_names"
+    )
+  }
+
+  f_arg <- .f_arg # for cli interpolation, avoid dot prefix; # nolint: object_usage_linter
+  withCallingHandlers(
+    {
+      force(.f)
+    },
+    error = function(e) {
+      cli_abort(
+        c("Failed to convert {.code {f_arg}} to a slide computation.",
+          "*" = "If you were trying to use the formula interface,
+                 maybe you forgot a tilde at the beginning.",
+          "*" = "If you were trying to use the tidyeval interface,
+                 maybe you forgot to specify the name,
+                 e.g.: `my_output_col_name =`.  Note that `.col_names`
+                 is not supported.",
+          "*" = "If you were trying to use advanced features of the
+                 tidyeval interface such as `!! name_variable :=`,
+                 maybe you forgot the required leading comma.",
+          "*" = "Something else could have gone wrong; see below."
+        ),
+        parent = e,
+        call = .call,
+        class = "epiprocess__as_slide_computation__error_forcing_.f"
+      )
+    }
+  )
 
   if (rlang::is_quosures(.f)) {
     quosures <- rlang::quos_auto_name(.f) # resolves := among other things
@@ -463,10 +502,10 @@ as_slide_computation <- function(.f, ..., .ref_time_value_long_varnames, .ref_ti
     }
 
     if (length(.f) > 2) {
-      cli_abort("{.code {arg}} must be a one-sided formula",
+      cli_abort("{.code {f_arg}} must be a one-sided formula",
         class = "epiprocess__as_slide_computation__formula_is_twosided",
         epiprocess__f = .f,
-        call = call
+        .call = .call
       )
     }
     if (rlang::dots_n(...) > 0L) {
@@ -486,7 +525,7 @@ as_slide_computation <- function(.f, ..., .ref_time_value_long_varnames, .ref_ti
         class = "epiprocess__as_slide_computation__formula_has_no_env",
         epiprocess__f = .f,
         epiprocess__f_env = env,
-        arg = arg, call = call
+        .f_arg = .f_arg, .call = .call
       )
     }
 
@@ -513,26 +552,32 @@ as_slide_computation <- function(.f, ..., .ref_time_value_long_varnames, .ref_ti
     class = "epiprocess__as_slide_computation__cant_convert_catchall",
     epiprocess__f = .f,
     epiprocess__f_class = class(.f),
-    arg = arg,
-    call = call
+    .f_arg = .f_arg,
+    .call = .call
   )
 }
 
 #' @rdname as_slide_computation
+#' @importFrom rlang caller_arg caller_env
 #' @keywords internal
-as_time_slide_computation <- function(.f, ...) {
+as_time_slide_computation <- function(.f, ..., .f_arg = caller_arg(.f), .call = caller_env()) {
   as_slide_computation(
     .f, ...,
+    .f_arg = .f_arg,
+    .call = .call,
     .ref_time_value_long_varnames = ".ref_time_value",
     .ref_time_value_label = "reference time value"
   )
 }
 
 #' @rdname as_slide_computation
+#' @importFrom rlang caller_arg caller_env
 #' @keywords internal
-as_diagonal_slide_computation <- function(.f, ...) {
+as_diagonal_slide_computation <- function(.f, ..., .f_arg = caller_arg(.f), .call = caller_env()) {
   as_slide_computation(
     .f, ...,
+    .f_arg = .f_arg,
+    .call = .call,
     .ref_time_value_long_varnames = c(".version", ".ref_time_value"),
     .ref_time_value_label = "version"
   )

man/as_slide_computation.Rd

@@ -0,0 +1,40 @@
+# as_slide_computation raises errors as expected
+
+    Code
+      toy_edf %>% group_by(geo_value) %>% epi_slide(.window_size = 7, mean,
+        .col_names = "value")
+    Condition
+      Error in `epi_slide()`:
+      ! `epi_slide` and `epix_slide` do not support `.col_names`; consider:
+      * using `epi_slide_mean`, `epi_slide_sum`, or `epi_slide_opt`, if applicable
+      * using `.f = ~ .x %>% dplyr::reframe(across(your_col_names, list(your_func_name = your_func)))`
+
+---
+
+    Code
+      toy_edf %>% group_by(geo_value) %>% epi_slide(.window_size = 7, tibble(
+        slide_value = mean(.x$value)))
+    Condition
+      Error in `epi_slide()`:
+      ! Failed to convert `tibble(slide_value = mean(.x$value))` to a slide computation.
+      * If you were trying to use the formula interface, maybe you forgot a tilde at the beginning.
+      * If you were trying to use the tidyeval interface, maybe you forgot to specify the name, e.g.: `my_output_col_name =`.  Note that `.col_names` is not supported.
+      * If you were trying to use advanced features of the tidyeval interface such as `!! name_variable :=`, maybe you forgot the required leading comma.
+      * Something else could have gone wrong; see below.
+      Caused by error:
+      ! object '.x' not found
+
+---
+
+    Code
+      toy_archive %>% epix_slide(tibble(slide_value = mean(.x$value)))
+    Condition
+      Error in `epix_slide()`:
+      ! Failed to convert `.f` to a slide computation.
+      * If you were trying to use the formula interface, maybe you forgot a tilde at the beginning.
+      * If you were trying to use the tidyeval interface, maybe you forgot to specify the name, e.g.: `my_output_col_name =`.  Note that `.col_names` is not supported.
+      * If you were trying to use advanced features of the tidyeval interface such as `!! name_variable :=`, maybe you forgot the required leading comma.
+      * Something else could have gone wrong; see below.
+      Caused by error:
+      ! object '.x' not found
+

tests/testthat/_snaps/utils.md

@@ -230,6 +230,37 @@ test_that("as_slide_computation raises errors as expected", {
   expect_error(as_time_slide_computation(5),
     class = "epiprocess__as_slide_computation__cant_convert_catchall"
   )
+
+  # helper to make initial snapshots less error-prone:
+  expect_error_snapshot <- function(x, class) {
+    x_quo <- rlang::enquo(x)
+    rlang::inject(expect_error(!!x_quo, class = class)) # quick sanity check on class
+    rlang::inject(expect_snapshot(!!x_quo, error = TRUE)) # don't need cnd_class = TRUE since checked above
+  }
+
+  # If `.f` doesn't look like tidyeval and we fail to force it, then we hint to
+  # the user some potential problems:
+  toy_edf <- tibble(geo_value = 1, time_value = c(1, 2), value = 1:2) %>%
+    as_epi_df(as_of = 1)
+  toy_archive <- tibble(version = c(1, 2, 2), geo_value = 1, time_value = c(1, 1, 2), value = 1:3) %>%
+    as_epi_archive()
+  expect_error_snapshot(
+    toy_edf %>%
+      group_by(geo_value) %>%
+      epi_slide(.window_size = 7, mean, .col_names = "value"),
+    class = "epiprocess__as_slide_computation__given_.col_names"
+  )
+  expect_error_snapshot(
+    toy_edf %>%
+      group_by(geo_value) %>%
+      epi_slide(.window_size = 7, tibble(slide_value = mean(.x$value))),
+    class = "epiprocess__as_slide_computation__error_forcing_.f"
+  )
+  expect_error_snapshot(
+    toy_archive %>%
+      epix_slide(tibble(slide_value = mean(.x$value))),
+    class = "epiprocess__as_slide_computation__error_forcing_.f"
+  )
 })
 
 test_that("as_slide_computation works", {