Geom aesthetics based on theme

[teunbrand]

This PR aims to partially fix #2239 and is intended to replace #2749. It is a work in progress as a few things might need to happen before this is viable (more at the end about this).

The main gist is that there is a from_theme() function that works in a similar fashion to after_stat()/after_scale()/stage(). During Geom$use_defaults(), this will be used for masked evaluation from a new theme element. See example below for a quick demo:

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

p <- ggplot(mpg, aes(displ, hwy)) +
  theme(geom = element_geom(ink = "purple"))

p + geom_point(aes(colour = from_theme(ink)))

It also works from the geom defaults, but this isn't implemented in any geom yet, as a few geoms (notably geom_sf()) are recalcitrant to this method.

update_geom_defaults("point", aes(colour = from_theme(ink)))

p + geom_point()

^{Created on 2024-04-08 with reprex v2.1.0}

There are a few topics for discussion that mostly mirror those in #2749.

• Current colour options in element_geom() are ink, paper and accent. You can think of ink and paper as foreground and background respectively and could be renamed fg and bg if that is deemed more intuitive.
• The main reason I didn't went with colour_1, colour_2, fill_1, fill_2 etc as discussed in #2749, is because from_theme() could allow you to mix colours on the spot and you wouldn't need a large number of colours to account for all the different grayscale defaults in the geoms. See also next point.
• I would like a colour mixing function so that we could hypothetically do the following for e.g. GeomRect:

  default_aes = list(
    ...,
    fill = from_theme(col_mix(ink, paper, amount = 0.35))
  )
  

  This probably has a more comfortable home in the {scales} package, as I think we'd need {farver} for this and ggplot2 only indirectly depends on {farver}.
• geom_sf() performs typical Geom tasks (like setting defaults) in the sf_grob() and would require a bit of refactoring to wire this geom up directly.

[teunbrand]

Update: added text parameters to element_geom(), which are set when using complete themes. base_family and base_size populate these parameters.

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

ggplot(mtcars, aes(wt, mpg, label = rownames(mtcars))) +
  geom_text() +
  theme_gray(base_family = "Montserrat", base_size = 22)

^{Created on 2024-04-17 with reprex v2.1.0}

[teunbrand]

This is in a useable state now, with almost all geoms completing colour/fill/linewidth/family/(font)size defaults from the theme. Because from_theme() can have expressions, we can generate the whole grayscale gradient by mixing the paper (white) and ink (black) settings.

What isn't done up to this point:

• geom_sf(), it would need #5834 for it to work.
• There is a shim for colour mixing, which we can remove when https://github.com/r-lib/scales/pull/424 is accepted.
• There is a failing test I'm hesitant to change because I'm unsure about what the complete attribute exactly entails in a <theme> object. For example I don't know why we would expect a complete theme in the line below:

https://github.com/tidyverse/ggplot2/blob/9243e2f744e8531632f5ce9a51542530fdc62052/tests/testthat/test-theme.R#L224

Aside from these points, I'd be happy for any feedback on the direction, so I'm un-WIP-ping this PR.

[teunbrand]

Small update:

• geom_sf() now has themeable defaults.
• Failling test was removed. Assumption earlier was that theme wasn't built/completed after ggplot_build(), and now it is.

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

nc <- sf::st_read(system.file("shape/nc.shp", package = "sf"), quiet = TRUE)

ggplot(nc) +
  geom_sf() +
  theme(
    geom = element_geom(ink = "forestgreen")
  )

^{Created on 2024-04-29 with reprex v2.1.0}

[teunbrand]

I've added pointsize and pointshape to element_geom() and propagated these to the relevant geoms. This had a visual change in some snapshots, due to pointsize scaling with the theme's base_size argument, but this is somewhat intended. For geom_boxplot(), I had to set the outlier.shape and outlier.size defaults to NULL in order to retrieve these from the GeomBoxplot$default_aes field.

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

ggplot(mpg, aes(class, hwy)) +
  geom_boxplot() +
  theme(geom = element_geom(
    pointsize = 1, pointshape = 3,
    ink = "red", paper = "grey80"
  ))

^{Created on 2024-04-30 with reprex v2.1.0}

I think this is now ready for proper review.

[teunbrand]

The paper colour is essentially for when you have a dark theme. I can't really demonstrate directly as {ggdark} overrides geom defaults, but getting a rough grasp of it:

It is very hard to read out these boxplots:

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

# Very simplified dark theme
dark_theme <- theme(
  plot.background = element_rect(fill = "black"),
  panel.background = element_rect(fill = "grey10"),
  axis.text = element_text(colour = "white"),
  axis.ticks = element_line(colour = "white"),
  panel.grid = element_line(colour = "grey5"),
  axis.title = element_text(colour = "white")
)

ggplot(mpg, aes(class, displ)) +
  geom_boxplot() +
  dark_theme

But it is way easier when you swap light/dark in ink/paper:

last_plot() + theme(geom = element_geom(ink = "white", paper = "grey10"))

^{Created on 2024-04-30 with reprex v2.1.0}

[teunbrand]

Addional idea that popped up, but probably best for a different PR; We could have the entire theme be mixtures of ink and paper so that it is really trivial to make a darkmode plot.

[yutannihilation]

Aside, I'd really like @ clauswilke and @ yutannihilation to have a look at this as they were involved in the superseded PR

I hope I can have time to take a look! By the way, which PR do you mean? I might get lost.

[teunbrand]

Thanks! The previous PR for tackling this issue is https://github.com/tidyverse/ggplot2/pull/2749.

[yutannihilation]

Oh, thanks. I can't remember how I involved in https://github.com/tidyverse/ggplot2/pull/2749, so I was wondering if there was a different one. But, that's fine. I'll try to get the context...!

[teunbrand]

TODO: Taken from #5886:

we should also keep at least two fonts in the geom defaults inside themes

[teunbrand]

As in the issue that Hiroaki linked, there might be some revdep fallout for packages that want to compare values from the default aesthetics. We can consider doing a proper revdepcheck to assess the scope of the problem.

[yutannihilation]

As in the issue that Hiroaki linked, there might be some revdep fallout for packages that want to compare values from the default aesthetics

Yeah, while the linked one is simply my fault, I was wondering whether it can be any problem or not to lose access to the actual values of default_aess.

[thomasp85]

I agree, but I think this is a case of not being able to have a cake and eat it. Here I think the pro's outweigh the con's

[thomasp85]

result of revue-checks have been pushed. It's north of 200 breakages so definitely a large impact regardless of the usefulness of this.

Can you go through the failures and see if there are aspects of this PR that can alleviate some of the issues. Once we are certain with the implementation and have an overview over which packages needs fixing upstream we can merge it in and provide PRs to these

[thomasp85]

@teunbrand where are we on this?

[teunbrand]

@thomasp85 I'd like to merge #5933 in to main, as that was one of the fixable problems from the last revdepcheck alongside #5932. When that is done, I'd like to merge main into this PR and run another revdepcheck on this PR.

[teunbrand]

Alright this is ready for revdep, but given past revdepchecks, I'd guess most of them will be unrelated to this PR

[teunbrand]

Suggestion raised elsewhere by @EvaMaeRey:

It would only be nice to distinguish between polygon linewidths (like filled point's stroke) and line/path/etc linewidths

I think this may make more sense than the current thin and thick. Currently thick is rarely used and we could apply calculations inside from_theme() anyway in those cases.

[teunbrand]

So I've tried to figure out why the revdep failures exist and here is a short description of failures that occurred more than twice in order of frequency. Here is my digestion of the issues:

• Failures via {plotly} (60 times)
  • Partially due to this PR but also due to #5631 in that some theme elements are not calculated that should be. I think this is best fixed by sending a PR to {plotly} fixing these issues. EDIT: https://github.com/plotly/plotly.R/pull/2368
• Missing plot labels (38 times)
  • Not related to this PR, but relates to #5879.
• Failures {patchwork} (35 times)
  • Not related to this PR, patch already merged (https://github.com/thomasp85/patchwork/pull/365)
• Failures via {ggdist} (19 times)
  • Related to this PR, due to {ggdist} implementing a custom use_defaults() that doesn't have theme in the signature. This is probably best solved by sending a PR to {ggdist}. EDIT: https://github.com/mjskay/ggdist/pull/241
• Failures via {ggnewscale} (16 times)
  • Not related to this PR, but relates to #5879. Probably best solved by sending a PR. EDIT: ggnewscale 0.5.0, which was just released on CRAN appears to not have any of these issues 🎉
• Unknown reasons (15 times)
  • The 'I don't know: I give up' category of failures. These packages failed for reasons beyond my skillset to diagnose or will take a thorough deep dive to diagnose.
• Accessing default values (8 times)
  • Packages using something like GeomPoint$default_aes$colour for whatever reason. These packages probably need a PR to wrap access to these default values.
• Using width = NULL as parameter to layer() (5 times)
  • Not related to this PR, but relates to #5807. Because width is now an aesthetic, it is incompatible with 0-length specifications, whereas this wasn't the case as parameter.
• False positives (4 times)
  • Failures due to installed package size sitting on the 5Mb threshold and such. They shouldn't be related to this PR or even ggplot2.
• Failures via {ggforce} (4 times)
  • Not related to this PR but to #5917
• Failures due to length(ggplot()) or names(ggplot()) (4 times)
  • Not related to this PR, discussed in #6008.
• Tests require updates (3 times)
  • Related to this PR, tests that require simple updates because "grey35" is now sometimes "#595959" and whatnot.
• Miscellaneous reasons (28 times)
  • 22 other reasons that are (almost) unique

[teunbrand]

The current status of this PR is as follows; I've tried to remove most friction between this PR and reverse dependencies. This included small tweaks to this PR, but also new PRs to e.g. {ggdist} and {plotly}. There are some inevitable changes that dependencies would have to make.

Some reverse dependencies will need to update some of their code to deal with default aesthetics that no longer are constants. To make this somewhat easier, I've made a get_geom_defaults() helper that resolves from_theme() aesthetics. I can still send out PRs to these packages if necessary. The packages are {ggpol}, {inTextSummaryTable}, {xaringanthemer}, {entropart}, {geomtextpath}, {ggdark}, {gghighlight}.

I don't think there is any more work to be done on this PR at the moment and I'd recommend merging this after review so that unforeseen issues can be reported.

[teunbrand]

Did we land on a decision on thin+thick, vs stroke+line (or something to that accord)?

I don't think we did, but I don't have strong feelings either way. What seems more appropriate to you?

Also, we should add linetype for completeness.

Will do!

[EvaMaeRey]

Did we land on a decision on thin+thick, vs stroke+line (or something to that accord)?

'outline' or 'border' instead of 'stroke' is probably more consistent with everyday terminology.

[teunbrand]

TODO: distinguish borderwidth / linewidth and also bordertype and linetype