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

Allow extra sources of pronouns in f_eval() #54

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lionel-
Copy link
Collaborator

@lionel- lionel- commented Mar 19, 2016

This adds an explicit argument to f_eval() to provide additional pronouns. Contrarily to data, the contents of these pronouns cannot be accessed directly and must be qualified with the relevant pronoun.

@hadley
Copy link
Owner

hadley commented Apr 8, 2016

Could you provide some motivation for this?

@lionel-
Copy link
Collaborator Author

lionel- commented Apr 8, 2016

I am writing a ggplot2 Stat that draws regression lines based on fitted models. That's useful to have in addition to StatSmooth because in this case the model parameters are estimated from the whole data and not just within groups, so it helps seeing how the model smoothes the predictions. To make it work I need to map the ggplot grouping aesthetics to the model inputs, which can mostly be figured out automatically except in one case.

It's often useful to cut a continuous predictor into discrete ranges to plot how two continuous predictors interact. In this case I need a way to map the levels of the binned predictor to a value on the original continuous scale. So the Stat has a match argument that takes a list of formulaes.

aes_patterns <- list(
  hp = levels(.aes$colour) ~ means_equal_sizes(.data$hp)
)

It's useful to have .env, .aes and .data sources here. I guess there must be other situations where clearly distinct data sources are available for a computation.

@hadley
Copy link
Owner

hadley commented Apr 8, 2016

Ok, I think that makes sense.

But I think you should use ... instead. Then there's a natural parallel with data = blah and foo = bar

@lionel-
Copy link
Collaborator Author

lionel- commented Apr 8, 2016

Should I rename data to .data then?

@hadley
Copy link
Owner

hadley commented Apr 8, 2016

I was thinking you'd automatically prefix with .

@lionel-
Copy link
Collaborator Author

lionel- commented Apr 8, 2016

ok, I thought that would feel a bit magical

@lionel-
Copy link
Collaborator Author

lionel- commented Apr 8, 2016

Here you go @hadley

@hadley
Copy link
Owner

hadley commented Apr 18, 2016

@kevinushey suggested an alternative approach:

To force the lookups of 'x', 'y' and 'z' to occur in df, you would probably write:

condition <- f_eval(~ (.data$x^2 + .data$y^2 + .data$z^2) < 1, df)

Or, you could potentially just do something like

condition <- f_eval(bind(~ (x^2 + y^2 + z^2) < 1, data = list(x, y, z)))

Or even just

    condition <- f_eval(~ (x^2 + y^2 + z^2) < 1, data = list(x, y, z))

as an alternate mechanism of denoting where particular symbols should be looked up.

I wonder if a generic bind would be a better solution here

@lionel-
Copy link
Collaborator Author

lionel- commented Apr 18, 2016

IIUC, I think these are orthogonal:

  • The bind function so the package author can manipulate variable lookup. Could possibly be extended to assign a hierarchy of sources for variable lookup (though that may be a bit redundant with environments).
  • The pronouns so the end user can explicitly bypass variable lookup.

@lionel-
Copy link
Collaborator Author

lionel- commented Apr 18, 2016

But I'm not sure I get it. I don't see the difference between your third example and the current approach. And the list is supposed to be named right?

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