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

... of across() are evaluated too early? #5813

Closed
lionel- opened this issue Mar 11, 2021 · 6 comments · Fixed by #5815
Closed

... of across() are evaluated too early? #5813

lionel- opened this issue Mar 11, 2021 · 6 comments · Fixed by #5815
Assignees
Labels
bug an unexpected problem or unintended behavior
Milestone

Comments

@lionel-
Copy link
Member

lionel- commented Mar 11, 2021

Not sure why this doesn't work:

library(dplyr)

tibble(x = tibble(foo = 1)) %>%
  mutate(across(
    everything(),
    mutate,
    foo = foo + 1
  ))
#> Error: Problem with `mutate()` input `..1`.
#> ✖ object 'foo' not found
#> ℹ Input `..1` is `(function (.cols = everything(), .fns = NULL, ..., .names = NULL) ...`.

Ideally across() would work the same way as map():

tibble(x = tibble(foo = 1)) %>%
  map(
    mutate,
    foo = foo + 1
  )
#> $x
#> # A tibble: 1 x 1
#>     foo
#>   <dbl>
#> 1     2
@romainfrancois
Copy link
Member

😞 it's again because of the instrumented "top across", e.g. regular across works fine:

library(dplyr, warn.conflicts = FALSE)

tibble(x = tibble(foo = 1)) %>%
  mutate(identity(across(
    everything(),
    mutate,
    foo = foo + 1
  )))
#> # A tibble: 1 x 1
#>   x$foo
#>   <dbl>
#> 1     2

Created on 2021-03-11 by the reprex package (v0.3.0)

@romainfrancois romainfrancois added the bug an unexpected problem or unintended behavior label Mar 11, 2021
@romainfrancois romainfrancois self-assigned this Mar 11, 2021
@romainfrancois
Copy link
Member

Also, should it say ?

Input `..1` is `dplyr:::top_across(...)`

@lionel-
Copy link
Member Author

lionel- commented Mar 11, 2021

I think it's referring to the inner mutate that is passed directly to across. Maybe top_across() evaluates (parts of?) the expansion too early and should instead recreate the same calls? Then the error would refer to the symbol mutate instead of the result of evaluating that symbol.

@romainfrancois
Copy link
Member

🤔 actually ... are meant to be evaluated early, this however works fine:

library(dplyr, warn.conflicts = FALSE)

tibble(x = tibble(foo = 1)) %>%
  mutate(across(
    everything(),
    ~mutate(.x, foo = foo + 1)
  ))
#> # A tibble: 1 x 1
#>   x$foo
#>   <dbl>
#> 1     2

Created on 2021-03-11 by the reprex package (v0.3.0)

@romainfrancois
Copy link
Member

nevermind, it's "just" that top_across() was eagerly evaluating ... with list(...) when it can be more lazy with enquos(...)

@romainfrancois romainfrancois added this to the 1.0.6 milestone Mar 11, 2021
@lionel-
Copy link
Member Author

lionel- commented Mar 11, 2021

Need to be careful about multiple evaluations though. If the mapped function is not a data masking function, the arguments inside ... must be evaluated only once. Maybe worth adding a unit test for that case.

The best thing is not to capture or collect ... in any way but just pass them on in the calls, and evaluate those calls in the proper environment where the dots are defined. It's also possible to "forward" dots by binding the ... symbol in an environment to the DOTSXP object of another environment. (I don't know if any of this applies here, just laying out the options in case it's useful).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants