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

Delay dplyr_quosures() auto naming until needed #6797

Merged

Conversation

DavisVaughan
Copy link
Member

This mostly affects filter(), because expressions are never named there but we have been auto naming them anyways.

library(dplyr)

df <- tibble(x = 1, y = 2:3)

bench::mark(filter(df, x == 1, y == 2), iterations = 10000)

# Before
#> # A tibble: 1 × 6
#>   expression                      min   median `itr/sec` mem_alloc `gc/sec`
#>   <bch:expr>                 <bch:tm> <bch:tm>     <dbl> <bch:byt>    <dbl>
#> 1 filter(df, x == 1, y == 2)    987µs   1.14ms      847.    1.35MB     13.7

# After
#> # A tibble: 1 × 6
#>   expression                      min   median `itr/sec` mem_alloc `gc/sec`
#>   <bch:expr>                 <bch:tm> <bch:tm>     <dbl> <bch:byt>    <dbl>
#> 1 filter(df, x == 1, y == 2)    653µs    835µs     1218.    1.75MB     12.9

There are some benefits for mutate() and summarise(). If an across() expansion occurs, then we previously had auto-named the expansion for nothing. This is less important though.


if (!is_named) {
# Will be auto-named by `dplyr_quosure_name()` only as needed
name <- quosure
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that we capture and store the original quosure is somewhat important here, as there are cases with pick() where we expand the quosure into something else, so we need a reference to the original one to be able to auto-name it correctly. I've added a test for this.

@DavisVaughan DavisVaughan merged commit 03c0684 into tidyverse:main Mar 21, 2023
@DavisVaughan DavisVaughan deleted the fix/delay-quosure-auto-name branch March 21, 2023 13:48
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.

2 participants