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

.data and .env need strict matching semantics #2591

Closed
hadley opened this issue Mar 29, 2017 · 8 comments
Closed

.data and .env need strict matching semantics #2591

hadley opened this issue Mar 29, 2017 · 8 comments
Assignees
Labels

Comments

@hadley
Copy link
Member

@hadley hadley commented Mar 29, 2017

Using $.data_source from rlang

@hadley
Copy link
Member Author

@hadley hadley commented Mar 29, 2017

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

@krlmlr
Copy link
Member

@krlmlr krlmlr commented Mar 30, 2017

@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-
Copy link
Member

@lionel- lionel- commented Mar 31, 2017

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
Copy link
Member Author

@hadley hadley commented Mar 31, 2017

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

@lionel-
Copy link
Member

@lionel- lionel- commented Apr 7, 2017

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
Copy link
Member Author

@hadley hadley commented Apr 7, 2017

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
Copy link
Member

@krlmlr krlmlr commented Apr 7, 2017

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
Copy link
Member

@krlmlr krlmlr commented Apr 8, 2017

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.

@krlmlr krlmlr closed this in #2627 Apr 11, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jun 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants