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

Fix across() and cur_data() to work sequentially #4912

Merged
merged 8 commits into from
Mar 3, 2020

Conversation

DavisVaughan
Copy link
Member

Closes #4907

This PR fixes both across() and cur_data() when used in sequential expressions.

This was actually incredibly hard to get right. This is complicated by the fact that we don't want to resolve "unused" columns, which would mark them as "used", but we still have to check their types in eval_select() if something like across(is.numeric) is called.

The idea here is to separate the non-group columns that are currently present in the data into two groups. The first group are unused columns, which we pull from the original data. The second are used columns, which we pull from the current bindings.

The good news is that, even though this is more expensive, it is part of the "set up" work that gets cached in #4909. I'll rework that PR on top of this

@DavisVaughan
Copy link
Member Author

I also discovered to my horror that if you c() two length-0 named lists together, you get an unnamed list back. This broke eval_select() (as it should), but luckily vec_c() works correctly.

library(vctrs)
library(tidyselect)
library(rlang)

x <- list(x = 1)
x <- x[0]
x
#> named list()

# WHY?
c(x, x)
#> list()

vec_c(x, x)
#> named list()

eval_select(expr(everything()), c(x, x))
#> Error: Can't select within an unnamed vector.

eval_select(expr(everything()), vec_c(x, x))
#> named integer(0)

Created on 2020-02-28 by the reprex package (v0.3.0)

@DavisVaughan DavisVaughan requested review from romainfrancois and hadley and removed request for romainfrancois February 28, 2020 20:34
R/across.R Outdated Show resolved Hide resolved
@DavisVaughan DavisVaughan requested a review from hadley March 2, 2020 15:25
hadley
hadley approved these changes Mar 2, 2020
@DavisVaughan DavisVaughan merged commit de9d5a4 into tidyverse:master Mar 3, 2020
@DavisVaughan DavisVaughan deleted the sequential-eval branch March 3, 2020 20:23
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.

across() doesn't work sequentially
3 participants