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

Confusing error message #805

Closed
jennybc opened this issue Nov 20, 2019 · 8 comments
Closed

Confusing error message #805

jennybc opened this issue Nov 20, 2019 · 8 comments
Labels
bug

Comments

@jennybc
Copy link
Member

jennybc commented Nov 20, 2019

If you forget to specify the col which unnest_wider() should act on, it complains that x is missing. Feels like that should be col. It's not entirely clear to me if this is a tidyr or tidyselect problem

library(tidyverse)

df <- tibble(
  z = list(list(letter = "a"), list(letter = "b"))
)

df %>% 
  unnest_wider(z)
#> # A tibble: 2 x 1
#>   letter
#>   <chr> 
#> 1 a     
#> 2 b

df %>% 
  unnest_wider()
#> Error: argument "x" is missing, with no default

Created on 2019-11-19 by the reprex package (v0.3.0.9001)

cc @lionel-

@lionel-
Copy link
Member

lionel- commented Nov 20, 2019

I think we should compact missing arguments the same way we compact NULL.

tidyselect::eval_select(quote(c(NULL, NULL)), iris)
#> named integer(0)

tidyselect::eval_select(quote(c(, )), iris)
#> Error: argument "x" is missing, with no default

But also tidyr should either give a default value of NULL to signal the argument is optional, or explicitly check that it is empty.

@jennybc
Copy link
Member Author

jennybc commented Nov 20, 2019

I've been poking around other tidyr functions and many of them display the same problem (unpack(), other rectangling functions). Basically whenever a missing, e.g., .col is immediately handled viaenquo(.col), it prevents the standard message 'argument ".col" is missing, with no default' and, by the time it's emitted, the complaint is about "x".

@hadley
Copy link
Member

hadley commented Nov 24, 2019

@lionel- the code is currently:

col <- tidyselect::vars_select(tbl_vars(data), !!enquo(col))

which will become:

col <- tidyselect::vars_select(tbl_vars(data), {{ col }})

So it seems to me that it should be tidyselect's (or {{'s?) responsibility to trigger the correct error

@lionel-
Copy link
Member

lionel- commented Nov 25, 2019

{{ cannot give an error with missing arguments because it is valid to defuse them (and they can be inspected with quo_is_missing()).

I was thinking tidyselect would not treat it as an error, but as an empty selection. This makes NULL and missing arguments consistent. Then it makes sense for tidyr to make these explicitly optional by taking a default of NULL. If tidyr prefers to keep this an error, it'd have to check that the argument is missing.

@hadley
Copy link
Member

hadley commented Nov 27, 2019

@lionel- so you think I should be doing if (missing(col)) abort("Argument col is missing with no default")? That seems like a weird gotcha.

@hadley
Copy link
Member

hadley commented Nov 27, 2019

Can you help me understand why it's different to this?

library(rlang)

x <- expr()
expr(!!x)
#> Error in enexpr(expr): argument "x" is missing, with no default

Created on 2019-11-26 by the reprex package (v0.3.0)

@lionel-
Copy link
Member

lionel- commented Nov 27, 2019

In your example the missing argument is not wrapped in a quosure. Normally it would be wrapped via enquo() or {{.

x <- quo()
expr(!!x)
#> <quosure>
#> expr: ^
#> env:  empty

@hadley
Copy link
Member

hadley commented Nov 28, 2019

Oh duh, yeah, still seems weird that I need to manually check that col exists.

@hadley hadley added the bug label Nov 28, 2019
@hadley hadley closed this as completed in 7a72898 Nov 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug
Projects
None yet
Development

No branches or pull requests

3 participants