Allow faceting by expressions in addition to column names

[DanRuderman]

@hadley I made some simple modifications to the environments that are passed to quoted_df that seemed to make expressions work using I(). I haven't done any of the documenting because I'd like you to have a look first and see what I have overlooked (can't believe it would be this easy...).

Example: ggplot(mtcars) + stat_bin(aes(mpg)) + facet_grid(I(gear+1) ~ I(2*carb))

[jiho]

Would this allow to use interaction of factors as facets? I often want to do things like

library("ggplot2")
ggplot(diamonds) + geom_point(aes(x=depth, y=table)) + facet_wrap(~color:clarity))
ggplot(diamonds) + geom_point(aes(x=depth, y=table)) + facet_wrap(~interaction(color, clarity))

instead of

ggplot(diamonds) + geom_point(aes(x=depth, y=table)) + facet_wrap(~color+clarity))

to avoid stacking the two facet labels and save space. The first two do not work, I have to compute the interaction beforehand, put it in a new column and then call ggplot.

[hadley]

@DanRuderman The environment really needs to come from the plot environment, which means it'll need to be threaded down through as extra arguments to these calls. Do you want to have a go at doing that?

@jiho yes!

[DanRuderman]

@hadley Yes. I look forward to the learning experience. Question is, do you look forward to the teaching experience? ;-)

[DanRuderman]

OK - I think I got the plot's environment passed around correctly. It would be great to have some tough test cases to make sure it's all working. Would you be able to provide some?

I can work on documentation in the next week or so.

Am I using the correct version of Roxygen? (5.0.1.9000)

[DanRuderman]

@jiho

ggplot(diamonds) + geom_point(aes(x=depth, y=table)) + facet_wrap(~color:clarity)

[jiho]

Cool! That would be great. In addition to making things like the above easier, it also makes the syntax more consistent between aes() calls and facets, which would both be able to use expressions. I get angry looks from students when I first tell them that they can use formulas everywhere and then have to say "except in facets" about 1h later in the class ;-)

[hadley]

@DanRuderman You'll need a test case like this:

df <- data.frame(x = 1:5, y = 1:5, z = 1)

f <- function() {
  g <- function(x) x + 2

  ggplot(df, aes(x, y)) +
    geom_point() + 
    facet_grid(. ~ g(x))
}
f()

[hadley]

Could you use released roxygen please?

[hadley]

@thomasp85 you should be aware of this since it touches the facetting code

Now that you've implemented this, I do wonder if it might be better to make a clean switch to using lazyeval for the facetting specification. That will require more changes to how facets are created, but will make the rest of the code simpler because the environment is carried around in the formula. I'm also not super confident about plyr::eval.quoted() because it was written well before I knew what I was doing.

Let me take a bit of a look.

[thomasp85]

I'm watching (and commented on the issue about the facet rewrite)

I would advice against merging this as it touches code that has been completely rewritten and it would thus lead to a lot of merge conflicts. Even so, the end result of this PR will make it much easier for me to implement it in the new system...

[hadley]

To convert to lazyeval will require a decent amount of work because facets can currently be a character vector, formula, or list. The formula is double-sided, and uses addition to specify components - that will require a special "parser" to convert to a list of individual formats (silently dropping . and stripping I(). That's not too much work, but probably out of scope for this PR, and I don't have the time to do it right now (since I'm supposed to be finishing R4DS).

@DanRuderman I think that means we probably won't merge this PR, but I really appreciate you showing how easy it is in principle!

[thomasp85]

Yes - it is much appreciated and I'll make sure that you'll be mentioned in the NEWS :-)

[thomasp85]

@hadley I can give you a heads up when the rewrite is in a state where a conversion to lazyeval might make sense and we can discuss then

[DanRuderman]

@hadley Sounds good. For the moment I'll tidy things up according to your above recommendations.

[thomasp85]

This PR has now been ported to #1633 - Thanks @DanRuderman

[DanRuderman]

@thomasp85 Bravo!

[lock]

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/