-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add default add to functions method #3106
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
Conversation
…s such as "ggplot(mtcars, aes(x = disp, y = mpg)) + geom_point". This follows the pattern in magrittr of allowing both "5 %>% sin" and "5 %>% sin()". This increased regularity may be helpful for new users. New example worked and tests passed after change.
Thanks for the idea. But,
On the other hand, I don't strongly disagree with this PR, but we need some discussion about the semantics, at least. |
I just thought I would put it out there. Not my decision, but it does have some principles behind it. Part of what inspired it was that students attempt to generalize from one to the other (and often both pipes an ggplot2 compositing are taught in the same course). I feel student experimentation should be rewarded, not punished. Plus the error message is confusing to the newcomer. library(ggplot2)
ggplot(mtcars, aes(x=cyl, y=disp)) + geom_point
#> Error: Don't know how to add geom_point to a plot Created on 2019-01-27 by the reprex package (v0.2.1) An error message that mentioned the type or class of the thing it could not add might be clearer than mentioning just the name used. There is a related semantic principle, I was just trying to be succinct in submitting the PR. The following code works: library("ggplot2")
a <- geom_point()
ggplot(mtcars, aes(x=cyl, y=disp)) + a Created on 2019-01-27 by the reprex package (v0.2.1) Notice it doesn't say "Error: Don't know how to add a to a plot". The effect comes from R's evaluation semantics, but to the outside user it looks related: a substitution from name to the value was performed. For the function evaluation the principle is related: we can think of the function as an object needed a de-reference, and in ggplot2's world the natural de-reference may be evaluation (with no arguments). There is some precedent for this sort of treatment. Many functional languages use zero-argument functions as "thunks" to control if/when evaluation happens. So this also represents a valid user controlled lazy evaluation opportunity. |
I can’t really figure out whether I like this idea or not. I cannot come up with any strong arguments against, but on the other hand I generally am against making piping and adding more alike as I think it will lead to greater confusion in the end... If the error is confusing we should def tackle that, but I’d like to hear from @hadley, @karawoo, and @clauswilke about this PR before we decide a course of action |
Same here. I can't point to any reason why it's obviously problematic, but my gut feeling says no. I'm not sure it'll help students if they think |
However, I agree that the error message is a problem. If the error message was the following, would that help?
|
I too think that blurring the distinction between |
I definitely don’t think it’s a good idea. A better error might be nice, but it’s not something I’ve seen widespread confusion with. |
I feel Claus's suggestion is compact enough, if we are to implement a better error.
Let this issue closed for now (until the need arises), in the hope that someday people will get unfamiliar with using |
This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/ |
Add ggplot_add.function() to plot-construction.r. This allows examples such as "ggplot(mtcars, aes(x = disp, y = mpg)) + geom_point". This follows the pattern in magrittr of allowing both "5 %>% sin" and "5 %>% sin()". This increased regularity may be helpful for new users. New example worked and tests passed after change.