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

Adding dplyr::select.list() to override utils::select.list() #4279

Closed
ColinFay opened this issue Mar 13, 2019 · 3 comments
Closed

Adding dplyr::select.list() to override utils::select.list() #4279

ColinFay opened this issue Mar 13, 2019 · 3 comments
Milestone

Comments

@ColinFay
Copy link

@ColinFay ColinFay commented Mar 13, 2019

Following the Twitter conversation from here.

As utils::select.list() is registered as an S3, calling dplyr::select() on a list calls select.list() without informing the user, who is faced with an interactive prompt asking them to do a selection. As far as I have understood this behavior is not intended.

I think this misuse can arise if you intend to write map_df() %>% select() and you end up writing map() %>% select().

Here's a suggested solution:

  • having a message to the user that they are redirected to utils::select.list()
#' @export
select.list <- function(.data, ...) {
  message("Calling `select()` on a list, redirecting to `utils::select.list()`")
  utils::select.list(choices = .data)
}
> list(a = 12) %>% dplyr::select()
Calling `select()` on a list, redirecting to `utils::select.list()`

1: 12

Selection: 1
$a
[1] 12

> list(a = 12) %>% utils::select.list()

1: 12

Selection: 1
$a
[1] 12

Note: related to #2817

@romainfrancois
Copy link
Member

@romainfrancois romainfrancois commented Mar 13, 2019

🤔 I'd rather have it abort. select.list() is not a method in the first place, signatures don't match, ...

if the user wants to call select.list() they they should call select.list() not select().

@ColinFay
Copy link
Author

@ColinFay ColinFay commented Mar 13, 2019

That was also my first idea, but

select.list <- function(.data, ...) {
  abort("`select()` doesn't handle lists.")
}

makes select.list() fail 😕

> list(a = 12) %>% dplyr::select()
Error: `dplyr::select()` doesn't handle lists.
Call `rlang::last_error()` to see a backtrace

> list(a = 12) %>% select.list()
Error: `dplyr::select()` doesn't handle lists.
Call `rlang::last_error()` to see a backtrace

> list(a = 12) %>% utils::select.list()

1: 12

Selection: 1
$a
[1] 12

(But maybe there's a workaround?)

@romainfrancois romainfrancois added this to the 0.8.2 milestone May 27, 2019
romainfrancois added a commit that referenced this issue May 27, 2019
romainfrancois added a commit that referenced this issue May 28, 2019
@lock
Copy link

@lock lock bot commented Mar 21, 2020

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/

@lock lock bot locked and limited conversation to collaborators Mar 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants