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

Should complete() and friends detect data masks in caller environment? #506

Closed
lionel- opened this issue Oct 17, 2018 · 6 comments
Closed
Labels
reprex needs a minimal reproducible example

Comments

@lionel-
Copy link
Member

lionel- commented Oct 17, 2018

And should lst() take an optional mask as argument? cc @krlmlr

https://community.rstudio.com/t/how-to-make-complete-nesting-work-with-quosures-and-tidyeval/16473/4

@lionel-
Copy link
Member Author

lionel- commented Oct 17, 2018

Related to dplyr::case_when() issues.

@krlmlr
Copy link
Member

krlmlr commented Oct 17, 2018

Are you suggesting that complete() and nesting() behave differently depending on the caller? This feels like opening a new can of worms to me, but I don't fully understand the implications yet.

How is this related to case_when()?

As for lst(), perhaps we could use sth like

eval_tidy(tibble::lst(!!!ensyms(...)), .data)

? Do you see other advantages of having an explicit data mask in lst() ?

@lionel-
Copy link
Member Author

lionel- commented Oct 17, 2018

Behaving differently based on the caller is often the way to implement lexically scoped behaviour, i.e. the right thing to do.

Passing the data mask is a way to pass the user's lexical context to lst() where the evaluation happens. But good suggestion about quoting lst(), I think this might just work!

eval_tidy(expr(tibble::lst(!!!quos(...))), mask)

@lionel-
Copy link
Member Author

lionel- commented Oct 17, 2018

It doesn't work because:

  • lst() does not forward the caller env to lst_quos(), which does not forward it to eval_tidy(). So the lexical context passed to eval_tidy() (default value of the env argument) is the execution env of tibble:::lst_quos().

  • Even if the caller env is correctly forwarded, lst_quos() also uses a data mask. So rlang would need to be able to combine data masks passed as data with data masks found in env. This is possible but would take some work.

In any case it'd be cleaner if tibble did forward the user context to eval_tidy().

@hadley
Copy link
Member

hadley commented Jan 4, 2019

Could you please inline the discussion from community, including a small reprex?

@hadley hadley added the reprex needs a minimal reproducible example label Jan 4, 2019
@batpigandme
Copy link
Contributor

Question was basically why nesting() doesn't work with quosures.

Now there is a better error message thrown that tells you what to do so, FWIW, I don't think it's really problematic at this point.

library(tidyverse)

d <- mtcars %>%
  mutate_at(vars(carb, am), as.factor)

# desired behaviour works with this syntax
d %>%
  group_by(carb, am) %>%
  tally() %>%
  complete(am, nesting(carb), fill = list(n = 0))
#> # A tibble: 12 x 3
#> # Groups:   carb [6]
#>    am    carb      n
#>    <fct> <fct> <dbl>
#>  1 0     1         3
#>  2 1     1         4
#>  3 0     2         6
#>  4 1     2         4
#>  5 0     3         3
#>  6 1     3         0

# in a function, `nest` does not work with a quosure
fnc <- function(group, nest, data) {
  group <- enquo(group)
  nest <- enquo(nest)

  data %>%
    group_by(!!group, !!nest) %>%
    tally() %>%
    complete(!!group, nesting(!!nest), fill = list(n = 0))
}

fnc(am, carb, d)
#> Error in eval_tidy(xs[[i]], unique_output): object 'carb' not found

# uses `ensym()` for nest rather than `enquo()`

fnc2 <- function(group, nest, data) {
  group <- enquo(group)
  nest <- ensym(nest)
  
  data %>%
    group_by(!!nest, !!group) %>%
    tally() %>%
    complete(!!group, nesting(!!nest), fill = list(n = 0))
}

fnc2(am, carb, d)
#> # A tibble: 12 x 3
#> # Groups:   carb [6]
#>    am    carb      n
#>    <fct> <fct> <dbl>
#>  1 0     1         3
#>  2 1     1         4
#>  3 0     2         6
#>  4 1     2         4
#>  5 0     3         3
#>  6 1     3         0

Created on 2019-01-04 by the reprex package (v0.2.1.9000)

@hadley hadley closed this as completed Jan 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reprex needs a minimal reproducible example
Projects
None yet
Development

No branches or pull requests

4 participants