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

Throw classed error if allow_empty = FALSE + add more context to error message by adding error_arg to eval_select() #350

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

olivroy
Copy link

@olivroy olivroy commented Apr 25, 2024

Closes #347

basically implement what was raised in #282 (comment)

Addresses part of #327

I have to say this fix is a bit naive and narrow and lacks a more general view.

pkgload::load_all() 
#> ℹ Loading tidyselect
# give a better error message if you don't want empty selection
 # if supplying `error_arg`
 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")))
#> Error in select_not_empty(mtcars, cols = starts_with("vs2")) : 
#>   `cols` must select at least one column.

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

Would it make sense to recognize it in eval_relocate()?

A version I had locally had error_arg = c(before_arg, after_arg), but couldn't tell if it was useful.

This implementation doesn't change the default, which mean it is opt-in.

Reason behind.

I was working on gt and discovered allow_empty and I thought I'd use it to throw messages, (without having to rethrow it again)

But I found that the error message Can't select empty items was not informative at all, so it is required to rethrow it or make tweaks in tidyselect.

So I thought it would be great if tidyselect had a mechanism to do this, and discovered it was proposed last time tidyselect was updated, but was never implemented.

@olivroy olivroy changed the title Error arg Throw classed error if allow_empty = FALSE fails + add more context by adding error_arg to eval_select() Apr 25, 2024
@olivroy olivroy changed the title Throw classed error if allow_empty = FALSE fails + add more context by adding error_arg to eval_select() Throw classed error if allow_empty = FALSE + add more context to error message by adding error_arg to eval_select() Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FR: Error on empty selection allow_empty = FALSE has informative class
1 participant