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

POC: ignore() for aesthetic evaluation. #5418

Closed
wants to merge 9 commits into from

Conversation

teunbrand
Copy link
Collaborator

@teunbrand teunbrand commented Sep 13, 2023

This PR explores #5290 (comment).

While #5290 tackles the same issue on the data side of things, this PR aims to use mechanics similar to after_scale().
Briefly, this PR introduces ignore() as an aesthetic evaluation modifier, which then causes scales to skip doing anything with or calculating anything from these modified aesthetics.

Here are some notes:

  • The information on which aesthetics are ignored are stored in Layer$computed_mapping. Scales have plenty of opportunities to read/write from/to data independent of the layer. This caused an issue on how to transfer that information to the scales.
  • The solution in this PR is to rename the ignored columns any time data interacts with scales. Scales match column names based on their aesthetics, so this effectively lets these ignored aesthetics be ignored. Any time the layer wants to compute anything on the data (stat, position, geom etc.), the columns are renamed back to their original names.
  • That renaming strategy kind-of works until we arrive at coord transformation, which is typically under control of a Layer$geom$draw_*() function, which needs the original names. The 'solution' in this PR is to temporarily redefine the definition of position aesthetics in the Layer$draw_geom() method.
  • I took some additional care in annotate() to preserve ignore()'d variables.

At a first glance, results are pretty similar to #5290.

devtools::load_all("~/packages/ggplot2") # This PR
#> ℹ Loading ggplot2

df <- data.frame(t = seq(0, 2*pi, length.out = 100))

ggplot(mpg) +
  geom_point(aes(displ, hwy, colour = factor(cyl))) +
  geom_path(
    data = df, 
    aes(x = ignore(cos(t) / 2.1 + 0.5), 
        y = ignore(sin(t) / 2.1 + 0.5),
        colour = ignore(2))
  ) +
  annotate(
    "text", x = ignore(0.5), y = ignore(0.5),
    label = "I'm in the middle",
    colour = 4, size = 10
  )

Created on 2023-09-13 with reprex v2.0.2

However, this is only the case for very straightforward layers that do not reparametrize anything. For example, if you'd have to translate x/width parameters to xmin/xmax parameters, this approach doesn't keep track of what should be ignored.

devtools::load_all("~/packages/ggplot2") # This PR
#> ℹ Loading ggplot2
df <- data.frame(x = c(0.3, 0.5, 0.7), y = c(1, 2, 3))

ggplot(df, aes(ignore(x), y)) +
  geom_col() +
  xlim(0, 10)

Created on 2023-09-13 with reprex v2.0.2

Whereas #5290's approach bakes the ignoring part into the data, which begets the intended effect for these bars.

devtools::load_all("~/packages/ggplot2") # PR #5290
df <- data.frame(x = c(0.3, 0.5, 0.7), y = c(1, 2, 3))

ggplot(df, aes(I(x), y)) +
  geom_col() +
  xlim(0, 10)

Created on 2023-09-13 with reprex v2.0.2

At this point, I think #5290 has more benefits than this approach.

@thomasp85
Copy link
Member

Remind me what part of #5290 that compelled us to explore this direction?

@teunbrand
Copy link
Collaborator Author

Well the #5290 touches a lot of places to make exceptions for AsIs objects that we thought might be simplified by using the stage()/after_stat()/after_scale() approach. I generally think it was good to explore, but I'd still prefer the approach in #5290. However, we might take the renaming approach from this PR and apply it to AsIs columns instead of ignore()'d aesthetics and the footprint of the PR might be a lot smaller and work well.

@thomasp85
Copy link
Member

Ok - let's discuss next meeting

@teunbrand
Copy link
Collaborator Author

Closing this in favour of #5477

@teunbrand teunbrand closed this Oct 12, 2023
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