Skip to content

Commit

Permalink
Minor tweaks to make sure error messages are better.
Browse files Browse the repository at this point in the history
  • Loading branch information
olivroy committed Oct 22, 2024
1 parent e063ac7 commit dbc1fe9
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 16 deletions.
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,5 @@ ByteCompile: true
Config/Needs/website: tidyverse/tidytemplate
Config/testthat/edition: 3
Encoding: UTF-8
RoxygenNote: 7.3.2
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.3.2
9 changes: 8 additions & 1 deletion R/eval-relocate.R
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,13 @@ eval_relocate <- function(expr,
allow_predicates <- allow_predicates && tidyselect_data_has_predicates(data)
data <- tidyselect_data_proxy(data)

error_arg <- NULL
if (!is.null(before)) {
error_arg <- before_arg
}
if (!is.null(after)) {
error_arg <- c(error_arg, after_arg)
}
expr <- as_quosure(expr, env = env)
sel <- eval_select_impl(
x = data,
Expand All @@ -88,7 +95,7 @@ eval_relocate <- function(expr,
allow_empty = allow_empty,
allow_predicates = allow_predicates,
type = "relocate",
error_arg = c(before_arg, after_arg),
error_arg = error_arg,
error_call = error_call
)

Expand Down
5 changes: 3 additions & 2 deletions R/eval-select.R
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@
#' 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 if `allow_empty = FALSE`.
#' Will give a better error message if the selection ends up empty.
#' @param error_arg Argument names for `expr`. These
#' are used in error messages. (You can use `"..."` if `expr = c(...)`).
#' For now, this is used when `allow_empty = FALSE`.
#' @inheritParams rlang::args_dots_empty
#'
#' @return A named vector of numeric locations, one for each of the
Expand Down
5 changes: 3 additions & 2 deletions man/eval_select.Rd

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

10 changes: 4 additions & 6 deletions tests/testthat/_snaps/eval-relocate.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,13 @@
---

Code
(expect_error(relocate_loc(mtcars, before = integer(), allow_empty = FALSE)))
Output
<error/tidyselect_error_empty_selection>
relocate_loc(mtcars, before = integer(), allow_empty = FALSE)
Condition <tidyselect_error_empty_selection>
Error in `relocate_loc()`:
! `before` and `after` must select at least one column.
Code
(expect_error(relocate_loc(mtcars, starts_with("z"), allow_empty = FALSE)))
Output
<error/tidyselect_error_empty_selection>
relocate_loc(mtcars, starts_with("z"), allow_empty = FALSE)
Condition <tidyselect_error_empty_selection>
Error in `relocate_loc()`:
! `before` and `after` must select at least one column.

Expand Down
9 changes: 5 additions & 4 deletions tests/testthat/test-eval-relocate.R
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,11 @@ test_that("can forbid empty selections", {
test_that("can forbid empty selections", {
x <- c(a = 1, b = 2, c = 3)

expect_snapshot({
(expect_error(relocate_loc(mtcars, before = integer(), allow_empty = FALSE)))
(expect_error(relocate_loc(mtcars, starts_with("z"), allow_empty = FALSE)))
})
expect_snapshot(
error = TRUE, {
relocate_loc(mtcars, before = integer(), allow_empty = FALSE)
relocate_loc(mtcars, starts_with("z"), allow_empty = FALSE)
}, cnd_class = TRUE)
})


Expand Down

0 comments on commit dbc1fe9

Please sign in to comment.