Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use tidyselect error_arg argument to yield better messages when allow_rename = FALSE or allow_empty = FALSE #1578

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,11 @@ Imports:
rlang (>= 1.1.1),
stringr (>= 1.5.0),
tibble (>= 2.1.1),
tidyselect (>= 1.2.1),
tidyselect (>= 1.2.1.9000),
utils,
vctrs (>= 0.5.2)
Remotes:
r-lib/tidyselect
Suggests:
covr,
data.table,
Expand Down
2 changes: 1 addition & 1 deletion R/drop-na.R
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ drop_na.data.frame <- function(data, ...) {
# Use all columns if no `...` are supplied
cols <- data
} else {
vars <- tidyselect::eval_select(expr(c(!!!dots)), data, allow_rename = FALSE)
vars <- tidyselect::eval_select(expr(c(!!!dots)), data, allow_rename = FALSE, error_arg = "...")
cols <- data[vars]
}

Expand Down
3 changes: 2 additions & 1 deletion R/fill.R
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ fill.data.frame <- function(data,
vars <- names(tidyselect::eval_select(
expr = expr(c(...)),
data = data,
allow_rename = FALSE
allow_rename = FALSE,
error_arg = "..."
))

.direction <- arg_match0(
Expand Down
1 change: 1 addition & 0 deletions R/pack.R
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ unpack <- function(data,
expr = enquo(cols),
data = data,
allow_rename = FALSE,
error_arg = "cols",
error_call = error_call
)
cols <- out[cols]
Expand Down
6 changes: 2 additions & 4 deletions R/pivot-long.R
Original file line number Diff line number Diff line change
Expand Up @@ -364,14 +364,12 @@ build_longer_spec <- function(data,
expr = enquo(cols),
data = data[unique(names(data))],
allow_rename = FALSE,
allow_empty = FALSE,
error_arg = "cols",
error_call = error_call
)
cols <- names(cols)

if (length(cols) == 0) {
cli::cli_abort("{.arg cols} must select at least one column.", call = error_call)
}

if (is.null(names_prefix)) {
names <- cols
} else {
Expand Down
7 changes: 6 additions & 1 deletion R/pivot-wide.R
Original file line number Diff line number Diff line change
Expand Up @@ -486,13 +486,15 @@ build_wider_spec <- function(data,
data,
allow_rename = FALSE,
allow_empty = FALSE,
error_arg = "names_from",
error_call = error_call
)
values_from <- tidyselect::eval_select(
enquo(values_from),
data,
allow_rename = FALSE,
allow_empty = FALSE,
error_arg = "values_from",
error_call = error_call
)

Expand Down Expand Up @@ -562,13 +564,15 @@ build_wider_id_cols_expr <- function(data,
enquo(names_from),
data,
allow_rename = FALSE,
error_arg = "names_from",
error_call = error_call
)

values_from <- tidyselect::eval_select(
enquo(values_from),
data,
allow_rename = FALSE,
error_arg = "values_from",
error_call = error_call
)

Expand Down Expand Up @@ -603,6 +607,7 @@ select_wider_id_cols <- function(data,
enquo(id_cols),
data,
allow_rename = FALSE,
error_arg = "id_cols",
error_call = error_call
),
vctrs_error_subscript_oob = function(cnd) {
Expand Down Expand Up @@ -631,7 +636,7 @@ rethrow_id_cols_oob <- function(cnd, names_from_cols, values_from_cols, call) {
stop_id_cols_oob <- function(i, arg, call) {
cli::cli_abort(
c(
"`id_cols` can't select a column already selected by `{arg}`.",
"{.arg id_cols} can't select a column already selected by `{arg}`.",
i = "Column `{i}` has already been selected."
),
parent = NA,
Expand Down
1 change: 1 addition & 0 deletions R/separate-longer.R
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ map_unchop <- function(data, cols, fun, ..., .keep_empty = FALSE, .error_call =
data = data,
allow_rename = FALSE,
allow_empty = FALSE,
error_arg = "cols",
error_call = .error_call
)
col_names <- names(cols)
Expand Down
7 changes: 6 additions & 1 deletion R/separate-rows.R
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,12 @@ separate_rows.data.frame <- function(data,
check_string(sep)
check_bool(convert)

vars <- tidyselect::eval_select(expr(c(...)), data, allow_rename = FALSE)
vars <- tidyselect::eval_select(
expr(c(...)),
data,
allow_rename = FALSE,
error_arg = "..."
)
vars <- names(vars)

out <- purrr::modify_at(data, vars, str_split_n, pattern = sep)
Expand Down
1 change: 1 addition & 0 deletions R/separate-wider.R
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,7 @@ map_unpack <- function(data, cols, fun, names_sep, names_repair, error_call = ca
data = data,
allow_rename = FALSE,
allow_empty = FALSE,
error_arg = "cols",
error_call = error_call
)
col_names <- names(cols)
Expand Down
2 changes: 1 addition & 1 deletion R/unite.R
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ unite.data.frame <- function(data, col, ..., sep = "_", remove = TRUE, na.rm = F
if (dots_n(...) == 0) {
selection <- set_names(seq_along(data), names(data))
} else {
selection <- tidyselect::eval_select(expr(c(...)), data, allow_rename = FALSE)
selection <- tidyselect::eval_select(expr(c(...)), data, allow_rename = FALSE, error_arg = "...")
}

empty_selection <- length(selection) == 0L
Expand Down
2 changes: 1 addition & 1 deletion R/unnest-longer.R
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ unnest_longer <- function(data,

error_call <- current_env()

cols <- tidyselect::eval_select(enquo(col), data, allow_rename = FALSE)
cols <- tidyselect::eval_select(enquo(col), data, allow_rename = FALSE, error_arg = "col")
col_names <- names(cols)
n_col_names <- length(col_names)

Expand Down
2 changes: 1 addition & 1 deletion R/unnest-wider.R
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ unnest_wider <- function(data,

error_call <- current_env()

cols <- tidyselect::eval_select(enquo(col), data, allow_rename = FALSE)
cols <- tidyselect::eval_select(enquo(col), data, allow_rename = FALSE, error_arg = "col")
col_names <- names(cols)

for (i in seq_along(cols)) {
Expand Down
3 changes: 2 additions & 1 deletion R/unnest.R
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,8 @@ unnest.data.frame <- function(data,
cols <- tidyselect::eval_select(
expr = enquo(cols),
data = data,
allow_rename = FALSE
allow_rename = FALSE,
error_arg = "cols"
)
cols <- unname(cols)

Expand Down
1 change: 1 addition & 0 deletions tests/testthat/_snaps/pack.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@
Condition
Error in `unpack()`:
! Can't rename variables in this context.
i `cols` can't be renamed.

# unpack() validates its inputs

Expand Down
3 changes: 2 additions & 1 deletion tests/testthat/_snaps/pivot-long.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
Code
(expect_error(pivot_longer(iris, matches("foo"))))
Output
<error/rlang_error>
<error/tidyselect_error_empty_selection>
Error in `pivot_longer()`:
! `cols` must select at least one column.

Expand All @@ -80,6 +80,7 @@
Condition
Error in `pivot_longer()`:
! Can't rename variables in this context.
i `cols` can't be renamed.

# `names_to` is validated

Expand Down
9 changes: 5 additions & 4 deletions tests/testthat/_snaps/pivot-wide.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,19 +53,19 @@
(expect_error(pivot_wider(df, names_from = starts_with("foo"), values_from = val))
)
Output
<error/rlang_error>
<error/tidyselect_error_empty_selection>
Error in `pivot_wider()`:
! Must select at least one item.
! `names_from` must select at least one column.

# `values_from` must identify at least 1 column (#1240)

Code
(expect_error(pivot_wider(df, names_from = key, values_from = starts_with("foo")))
)
Output
<error/rlang_error>
<error/tidyselect_error_empty_selection>
Error in `pivot_wider()`:
! Must select at least one item.
! `values_from` must select at least one column.

# `values_fn` emits an informative error when it doesn't result in unique values (#1238)

Expand Down Expand Up @@ -183,6 +183,7 @@
Condition
Error in `pivot_wider()`:
! Can't rename variables in this context.
i `id_cols` can't be renamed.

# `id_expand` is validated

Expand Down
1 change: 1 addition & 0 deletions tests/testthat/_snaps/unnest.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
Condition
Error in `unnest()`:
! Can't rename variables in this context.
i `cols` can't be renamed.

# cols must go in cols

Expand Down
Loading