tidyverse / dplyr

dplyr: A grammar of data manipulation
https://dplyr.tidyverse.org/
Other
4.77k stars 2.12k forks source link

.data and .env need strict matching semantics #2591

Closed hadley closed 7 years ago

hadley commented 7 years ago

Using $.data_source from rlang

hadley commented 7 years ago

filter(mtcars, .data$blah > 0) should return a clean error saying that blah does not exist.

krlmlr commented 7 years ago

@lionel-: For this, I'd like to call data_source.environment() from the C++ code but avoid cloning the environment. It might be worthwhile to provide a native entry point, too. Please advise.

lionel- commented 7 years ago

maybe we shouldn't clone the environment?

@hadley I think the cloning dates back to lazyeval, do we need to clone? If it's to avoid leaking the environment, we could create a child.

hadley commented 7 years ago

Yeah, a child is probably better. I just wanted to avoid in place modification like .env$x <- 10

lionel- commented 7 years ago

if we create a child, we can't access variables with .env$x anymore unless we do something special in the $ method. But maybe that's for the best because $ subsetting won't work within magrittr pipelines either.

And we probably should not focus on .env in tidyeval documentation since unquoting solves the issue of taking values from the dynamic environment rather than the overscoped data. .env may still be useful for expert uses, such as retrieving a value at evaluation time rather than immediately.

hadley commented 7 years ago

TBH, I'd be fine with ignoring .env for now, but it's important that we have strict matching for .data for this release.

krlmlr commented 7 years ago

To move forward, I'll replicate rlang:::new_data_source() in C++ in dplyr for now, we can clean up later on. I think I can make both .data and .env a data_source object with a proper error message.

krlmlr commented 7 years ago

re in-place modification: Do we want to define $<-.data_source() in rlang to throw a sensible error? We might get away without cloning then.