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

FR: Error on empty selection allow_empty = FALSE has informative class #347

Open
eutwt opened this issue Nov 30, 2023 · 1 comment · May be fixed by #350
Open

FR: Error on empty selection allow_empty = FALSE has informative class #347

eutwt opened this issue Nov 30, 2023 · 1 comment · May be fixed by #350

Comments

@eutwt
Copy link
Contributor

eutwt commented Nov 30, 2023

Many of the eval_select() errors have a class that lets you know what went wrong, but not the error you get on an empty selection with allow_empty = FALSE. This would be useful if people want to rethrow the error so that the "Must select at least one item" message is more informative in the context of their function, especially to disambiguate the source of the error if there are multiple tidy-select arguments (e.g. rethrow as "Must select at least one country").

library(tidyselect)

eval_select(quote(any_of('z')), list(x = 1), allow_empty = FALSE) |>
  rlang::catch_cnd() |> 
  class()
#> [1] "rlang_error" "error"       "condition"

eval_select(c(a = 'x'), list(x = 1), allow_rename = FALSE) |>
  rlang::catch_cnd() |> 
  class()
#> [1] "tidyselect:::error_disallowed_rename"
#> [2] "rlang_error"                         
#> [3] "error"                               
#> [4] "condition"

eval_select('z', list(x = 1)) |>
  rlang::catch_cnd() |> 
  class()
#> [1] "vctrs_error_subscript_oob" "vctrs_error_subscript"    
#> [3] "rlang_error"               "error"                    
#> [5] "condition"

eval_select(list(1), list(x = 1)) |>
  rlang::catch_cnd() |> 
  class()
#> [1] "vctrs_error_subscript_type" "vctrs_error_subscript"     
#> [3] "rlang_error"                "error"                     
#> [5] "condition"

Created on 2023-11-30 with reprex v2.0.2

@olivroy
Copy link

olivroy commented Apr 25, 2024

Could also improve the base message?

f <- function(dat, cols) {
  tidyselect::eval_select(
    expr = rlang::enquo(cols),
    data = dat,
    allow_empty = FALSE
  )
}
mtcars |> f(c(vs, mpg2))
#> Error in `f()`:
#> ! Can't select columns that don't exist.
#> ✖ Column `mpg2` doesn't exist.
mtcars |> f(c(tidyselect::starts_with("vs")))
#> vs 
#>  8
mtcars |> f(c(tidyselect::starts_with("vs2")))
#> Error in `f()`:
#> ! Must select at least one item.


mtcars |> tidyr::pivot_longer(tidyselect::starts_with("vs2"))
#> Error in `tidyr::pivot_longer()`:
#> ! `cols` must select at least one column.

Created on 2024-04-25 with reprex v2.1.0

Or add maybe an arg_nm?

so that it could throw a message like tidyr does natively?

cols must select at least one column

Currently, tidyr rethrows manually

https://github.com/tidyverse/tidyr/blob/c6c126a61f67a10b5ab9ce6bb1d9dbbb7a380bbd/R/pivot-long.R#L363-L373

But with my proposal,

# The call in tidyr could become 
 cols <- tidyselect::eval_select(
    expr = enquo(cols),
    data = data[unique(names(data))],
    allow_rename = FALSE,
    allow_empty = FALSE,
    error_arg = "cols",
    error_call = error_call
  )
#> `cols` must select at least one column 

error_arg was proposed in #282 (comment), is there plan to implement it?

Actually, the open issue is #327

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants