Skip to content

Commit

Permalink
Finalize implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
olivroy committed Apr 25, 2024
1 parent f6816ca commit 036d2e3
Show file tree
Hide file tree
Showing 10 changed files with 75 additions and 18 deletions.
5 changes: 5 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
7 changes: 5 additions & 2 deletions R/eval-relocate.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
8 changes: 7 additions & 1 deletion R/eval-select.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()`
Expand Down
14 changes: 13 additions & 1 deletion R/eval-walk.R
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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
)
}
Expand Down
8 changes: 7 additions & 1 deletion man/eval_select.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 7 additions & 10 deletions man/faq-selection-context.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions tests/testthat/_snaps/eval-relocate.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,19 +84,19 @@
Code
(expect_error(relocate_loc(x, allow_empty = FALSE)))
Output
<error/rlang_error>
<error/tidyselect:::error_disallowed_empty>
Error in `relocate_loc()`:
! Must select at least one item.
Code
(expect_error(relocate_loc(mtcars, integer(), allow_empty = FALSE)))
Output
<error/rlang_error>
<error/tidyselect:::error_disallowed_empty>
Error in `relocate_loc()`:
! Must select at least one item.
Code
(expect_error(relocate_loc(mtcars, starts_with("z"), allow_empty = FALSE)))
Output
<error/rlang_error>
<error/tidyselect:::error_disallowed_empty>
Error in `relocate_loc()`:
! Must select at least one item.

Expand Down
18 changes: 18 additions & 0 deletions tests/testthat/_snaps/eval-select.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions tests/testthat/test-eval-select.R
Original file line number Diff line number Diff line change
Expand Up @@ -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()))))
Expand Down

0 comments on commit 036d2e3

Please sign in to comment.