Skip to content

Commit

Permalink
Lint, tweak error messages, refactor class+snap tests, guide on .col_…
Browse files Browse the repository at this point in the history
…names
  • Loading branch information
brookslogan committed Oct 2, 2024
1 parent c5b5b26 commit f4ce52e
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 24 deletions.
33 changes: 28 additions & 5 deletions R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -358,18 +358,41 @@ 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, ..., .f_arg = caller_arg(.f), .call = caller_env(), .ref_time_value_long_varnames, .ref_time_value_label) {
f_arg <- .f_arg # for cli interpolation, avoid dot prefix
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 =`.",
"*" = "If you were trying to use advanced features of the tidyeval interface such as `!! name_variable :=`, you might have forgotten the required leading comma."
"*" = "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,
Expand Down
27 changes: 20 additions & 7 deletions tests/testthat/_snaps/utils.md
Original file line number Diff line number Diff line change
@@ -1,14 +1,26 @@
# as_slide_computation raises errors as expected

Code
toy_edf %>% group_by(geo_value) %>% epi_slide(.window_size = 6, tibble(
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 `as_slide_computation()`:
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 =`.
* If you were trying to use advanced features of the tidyeval interface such as `!! name_variable :=`, you might have forgotten the required leading comma.
* 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

Expand All @@ -17,11 +29,12 @@
Code
toy_archive %>% epix_slide(tibble(slide_value = mean(.x$value)))
Condition
Error in `as_slide_computation()`:
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 =`.
* If you were trying to use advanced features of the tidyeval interface such as `!! name_variable :=`, you might have forgotten the required leading comma.
* 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

26 changes: 14 additions & 12 deletions tests/testthat/test-utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -231,34 +231,36 @@ test_that("as_slide_computation raises errors as expected", {
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(
expect_error_snapshot(
toy_edf %>%
group_by(geo_value) %>%
epi_slide(.window_size = 6, tibble(slide_value = mean(.x$value))),
class = "epiprocess__as_slide_computation__error_forcing_.f"
epi_slide(.window_size = 7, mean, .col_names = "value"),
class = "epiprocess__as_slide_computation__given_.col_names"
)
expect_snapshot(
error = TRUE,
expect_error_snapshot(
toy_edf %>%
group_by(geo_value) %>%
epi_slide(.window_size = 6, tibble(slide_value = mean(.x$value)))
epi_slide(.window_size = 7, tibble(slide_value = mean(.x$value))),
class = "epiprocess__as_slide_computation__error_forcing_.f"
)
expect_error(
expect_error_snapshot(
toy_archive %>%
epix_slide(tibble(slide_value = mean(.x$value))),
class = "epiprocess__as_slide_computation__error_forcing_.f"
)
expect_snapshot(
error = TRUE,
toy_archive %>%
epix_slide(tibble(slide_value = mean(.x$value)))
)
})

test_that("as_slide_computation works", {
Expand Down

0 comments on commit f4ce52e

Please sign in to comment.