From 036d2e38c539330af5ec92e302e37e69760d5807 Mon Sep 17 00:00:00 2001 From: olivroy Date: Thu, 25 Apr 2024 12:56:46 -0400 Subject: [PATCH] Finalize implementation --- NEWS.md | 5 +++++ R/eval-relocate.R | 7 +++++-- R/eval-select.R | 8 +++++++- R/eval-walk.R | 14 +++++++++++++- R/utils.R | 2 ++ man/eval_select.Rd | 8 +++++++- man/faq-selection-context.Rd | 17 +++++++---------- tests/testthat/_snaps/eval-relocate.md | 6 +++--- tests/testthat/_snaps/eval-select.md | 18 ++++++++++++++++++ tests/testthat/test-eval-select.R | 8 ++++++++ 10 files changed, 75 insertions(+), 18 deletions(-) diff --git a/NEWS.md b/NEWS.md index 398f8ac1..36db39d2 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,10 @@ # tidyselect (development version) +* `eval_select(allow_empty = FALSE)` gains a new argument to yield a better error + message in case of empty selection (@olivroy, #327) + +* `eval_select(allow_empty = FALSE)` now throws a classed error message (@olivroy, #347). + # tidyselect 1.2.1 * Performance improvements (#337, #338, #339, #341) diff --git a/R/eval-relocate.R b/R/eval-relocate.R index 45d46b62..b0b16795 100644 --- a/R/eval-relocate.R +++ b/R/eval-relocate.R @@ -89,6 +89,7 @@ eval_relocate <- function(expr, allow_empty = allow_empty, allow_predicates = allow_predicates, type = "relocate", + error_arg = NULL, # TODO need to know which to use. can `before_arg` or `after_arg` be passed here? error_call = error_call ) @@ -122,7 +123,8 @@ eval_relocate <- function(expr, env = env, error_call = error_call, allow_predicates = allow_predicates, - allow_rename = FALSE + allow_rename = FALSE, + error_arg = before_arg ), arg = before_arg, error_call = error_call @@ -143,7 +145,8 @@ eval_relocate <- function(expr, env = env, error_call = error_call, allow_predicates = allow_predicates, - allow_rename = FALSE + allow_rename = FALSE, + error_arg = after_arg ), arg = after_arg, error_call = error_call diff --git a/R/eval-select.R b/R/eval-select.R index 2b9b424b..7ee2cc42 100644 --- a/R/eval-select.R +++ b/R/eval-select.R @@ -43,7 +43,8 @@ #' use predicates (i.e. in `where()`). If `FALSE`, will error if `expr` uses a #' predicate. Will automatically be set to `FALSE` if `data` does not #' support predicates (as determined by [tidyselect_data_has_predicates()]). -#' @param error_arg Argument name to include in error message. +#' @param error_arg Argument name to include in error message if `allow_empty = FALSE`. +#' Will give a better error message if the selection ends up empty. #' @inheritParams rlang::args_dots_empty #' #' @return A named vector of numeric locations, one for each of the @@ -104,6 +105,11 @@ #' # Note that the trick above works because `expr({{ arg }})` is the #' # same as `enquo(arg)`. #' +#' # give a better error message if you don't want empty selection +#' select_not_empty <- function(x, cols) { +#' eval_select(expr = enquo(cols), data = x, allow_empty = FALSE, error_arg = "cols") +#' } +#' try(select_not_empty(mtcars, cols = starts_with("vs2"))) #' #' # The evaluators return a named vector of locations. Here are #' # examples of using these location vectors to implement `select()` diff --git a/R/eval-walk.R b/R/eval-walk.R index caf5dc38..7d8a3f3d 100644 --- a/R/eval-walk.R +++ b/R/eval-walk.R @@ -131,7 +131,19 @@ ensure_named <- function(pos, check_empty <- function(x, allow_empty = TRUE, error_arg = NULL, call = caller_env()) { if (!allow_empty && length(x) == 0) { - cli::cli_abort("Must select at least one item.", call = call) + if (is.null(error_arg)) { + cli::cli_abort( + "Must select at least one item.", + call = call, + class = "tidyselect:::error_disallowed_empty" + ) + } else { + cli::cli_abort( + "{.or {.arg {error_arg}}} must select at least one column.", + call = call, + class = "tidyselect:::error_disallowed_empty" + ) + } } } diff --git a/R/utils.R b/R/utils.R index 2a66b618..3663c37e 100644 --- a/R/utils.R +++ b/R/utils.R @@ -8,6 +8,7 @@ select_loc <- function(x, allow_rename = TRUE, allow_empty = TRUE, allow_predicates = TRUE, + error_arg = NULL, error_call = current_env()) { check_dots_empty() @@ -21,6 +22,7 @@ select_loc <- function(x, allow_rename = allow_rename, allow_empty = allow_empty, allow_predicates = allow_predicates, + error_arg = error_arg, error_call = error_call ) } diff --git a/man/eval_select.Rd b/man/eval_select.Rd index 11fc8bc6..bced545c 100644 --- a/man/eval_select.Rd +++ b/man/eval_select.Rd @@ -76,7 +76,8 @@ is useful to implement purely selective behaviour.} in an empty selection. If \code{FALSE}, will error if \code{expr} yields an empty selection.} -\item{error_arg}{Argument name to include in error message.} +\item{error_arg}{Argument name to include in error message if \code{allow_empty = FALSE}. +Will give a better error message if the selection ends up empty.} } \value{ A named vector of numeric locations, one for each of the @@ -143,6 +144,11 @@ my_function <- function(.x, .expr, ...) { # Note that the trick above works because `expr({{ arg }})` is the # same as `enquo(arg)`. +# give a better error message if you don't want empty selection +select_not_empty <- function(x, cols) { + eval_select(expr = enquo(cols), data = x, allow_empty = FALSE, error_arg = "cols") +} +try(select_not_empty(mtcars, cols = starts_with("vs2"))) # The evaluators return a named vector of locations. Here are # examples of using these location vectors to implement `select()` diff --git a/man/faq-selection-context.Rd b/man/faq-selection-context.Rd index d4189790..860fbf80 100644 --- a/man/faq-selection-context.Rd +++ b/man/faq-selection-context.Rd @@ -13,23 +13,20 @@ Using a selection helper anywhere else results in an error: \if{html}{\out{
}}\preformatted{starts_with("foo") #> Error: #> ! `starts_with()` must be used within a *selecting* function. -#> i See -#> -#> for details. +#> i See for +#> details. mtcars[contains("foo")] #> Error: #> ! `contains()` must be used within a *selecting* function. -#> i See -#> -#> for details. +#> i See for +#> details. subset(mtcars, select = matches("foo")) #> Error: #> ! `matches()` must be used within a *selecting* function. -#> i See -#> -#> for details. +#> i See for +#> details. }\if{html}{\out{
}} If you see this error, you may have used a selection helper in the wrong @@ -38,7 +35,7 @@ argument name). Alternatively, you may be deliberately trying to reduce duplication in your code by extracting out a selection into a variable: \if{html}{\out{
}}\preformatted{my_vars <- c(name, species, ends_with("color")) -#> Error in eval(expr, envir, enclos): object 'name' not found +#> Error in eval(expr, envir, enclos): objet 'name' introuvable }\if{html}{\out{
}} To make this work you’ll need to do two things: diff --git a/tests/testthat/_snaps/eval-relocate.md b/tests/testthat/_snaps/eval-relocate.md index 977018e4..d2d9ad1e 100644 --- a/tests/testthat/_snaps/eval-relocate.md +++ b/tests/testthat/_snaps/eval-relocate.md @@ -84,19 +84,19 @@ Code (expect_error(relocate_loc(x, allow_empty = FALSE))) Output - + Error in `relocate_loc()`: ! Must select at least one item. Code (expect_error(relocate_loc(mtcars, integer(), allow_empty = FALSE))) Output - + Error in `relocate_loc()`: ! Must select at least one item. Code (expect_error(relocate_loc(mtcars, starts_with("z"), allow_empty = FALSE))) Output - + Error in `relocate_loc()`: ! Must select at least one item. diff --git a/tests/testthat/_snaps/eval-select.md b/tests/testthat/_snaps/eval-select.md index 3d1fdf79..13264cf1 100644 --- a/tests/testthat/_snaps/eval-select.md +++ b/tests/testthat/_snaps/eval-select.md @@ -66,6 +66,24 @@ Error in `select_loc()`: ! Must select at least one item. +# can forbid empty selections with informative error + + Code + select_loc(mtcars, allow_empty = FALSE, error_arg = "cols") + Condition + Error in `select_loc()`: + ! `cols` must select at least one column. + Code + select_loc(mtcars, integer(), allow_empty = FALSE, error_arg = "x") + Condition + Error in `select_loc()`: + ! `x` must select at least one column. + Code + select_loc(mtcars, starts_with("z"), allow_empty = FALSE, error_arg = "y") + Condition + Error in `select_loc()`: + ! `y` must select at least one column. + # eval_select() errors mention correct calls Code diff --git a/tests/testthat/test-eval-select.R b/tests/testthat/test-eval-select.R index 9218ce3e..9a3721e9 100644 --- a/tests/testthat/test-eval-select.R +++ b/tests/testthat/test-eval-select.R @@ -105,6 +105,14 @@ test_that("can forbid empty selections", { }) }) +test_that("can forbid empty selections with informative error", { + expect_snapshot(error = TRUE, { + select_loc(mtcars, allow_empty = FALSE, error_arg = "cols") + select_loc(mtcars, integer(), allow_empty = FALSE, error_arg = "x") + select_loc(mtcars, starts_with("z"), allow_empty = FALSE, error_arg = "y") + }) +}) + test_that("eval_select() errors mention correct calls", { f <- function() stop("foo") expect_snapshot((expect_error(select_loc(mtcars, f()))))