Superfluous guidance in "Creating a new theme" section of ggplot2 packages vignette?

[david-romano]

[Motivated by Posit Forum post Creating a new ggplot2 theme.]

The Creating a new theme section of the vignette on using ggplot2 in packages creates a new theme called theme_custom() and advises using a wrapper to apply the new theme:

default_theme <- function() {
  theme_custom()
}

mpg_drv_summary2 <- function() {
  mpg_drv_summary() + default_theme()
}

However, since theme_custom() is already a function, the reason for the wrapper in not clear. The motivation given is that:

[...i]f not, the theme object is stored in the compiled bytecode of the built package, which may or may not align with the installed version of ggplot2!

but my understanding is that this shouldn't happen unless, say, the package contained a call to theme_set().

Is the wrapper superfluous, or am I missing something?

[teunbrand]

Honestly, I'm a little surprised by this recommendation, or at least the wording of it. If you have a theme_custom() constructor, you wouldn't need a default_theme() wrapper.

I think the important bit to take away is that whenever you make a theme, always actually contruct it by running the contents through the theme() function. If you're making a package, don't save an already constructed theme to a variable or to disk, but have a function that builds the theme from stratch (or builds on a pre-existing theme).

[!WARNING] Don't do the following

my_package_theme <- theme_gray() + theme(<your settings>)

my_plotting_function <- function(...) {
  ggplot() + ... + my_package_theme
}

[!TIP] Use a theme constructor instead

my_package_theme <- function(...) theme_gray(...) + theme(<your setting>)

my_plotting_function <- function(...) {
  ggplot() + ... + my_package_theme()
}

An example of where this may go wrong is in https://github.com/csdaw/ggprism/issues/26, where a theme was saved to disk and loaded whenever requested. ggplot2 then deprecated the legend.text.align theme setting with fallbacks in place in the theme() function. But, because it was loaded from disk (and thus contained the deprecated element) and not run through theme(), the fallbacks didn't kick in. That theme kept throwing errors about elements that didn't exist and also affected packages downstream.

[teunbrand]

In any case, I think we should adjust this in the vignette, unless other contributors have more insights.