Allow default geom aesthetics to be set from theme

[dpseidel]

Motivation

The goal of this PR is to allow certain default aesthetics to be set by the plot theme. This can be used to achieve better default behaviours (e.g. plotting white points by default, instead of black, when using theme_dark()) and to allow user created themes to more easily control default aesthetics. This PR addresses #2239.

Basic Approach

The basic approach is to alter each Geom's default_aes from something like:

# alpha must default to NA to respect RGBA color specification
default_aes = aes(colour = "black", size = 0.5, linetype = 1, alpha = NA)

to something more like:

default_aes = aes(
  colour = theme$geom$col,
  fill = theme$geom$fill,
  linetype = 1,
  alpha = NA
)

and then evaluate the default_aes mappings in an environment conscious of plot$theme. Since aes() quotes all arguments, I currently I do this by wrapping default_aes in expr() and evaluating it inside geom$use_defaults. This in turn gets called to build each layer and the guides. Some minor adjustments were also made to handle incomplete themes (I'm confident there is a better way, consider this a temporary patch) and to maintain update_geom_defaults() functionality.

@hadley, I think this is still slightly different from the algorithm we originally discussed. You originally indicated that we should evaluate the mappings within the plot environment and then call something like on.exit(set_geom_theme(NULL)), to reset to the default expression. I so far have been unable to make this work, and I think it may require our evaluation to happen earlier in the code. If this is still what you have in mind, I can reconsider and refactor the code. I agree it could be more streamline to evaluate these defaults upfront rather than at multiple points in the build, I simply have yet to make that work.

Next Steps

This PR is a work in progress. Currently I have only added theme elements colour and fill and only updated the default_aes for GeomPoint and GeomRect, eventually all geoms will need to be updated. This will mean making some decisions about how to handle variability across geom defaults. For example, looking just at colour: the default for geom_smooth(), geom_contour(), geom_density2d(), and geom_quantile() is all #3366FF and the rest of the geoms default to colour = "black" (or grey20 for boxplots and violins but this can and probably should be black also). It would make sense to set the default to black in theme_grey(), but that either means that geom_smooth() et al. should not be coded to inherit from colour from theme() ever, or these geoms will also be forced to default to black. I lean towards leaving these special defaults hardcoded in these cases as to not change expected behaviour. This may also influence exactly which elements should be set in themes (eg. size has more meaningful variability across geoms that we may want to preserve). A full list of geom defaults is here. From my analysis, I suspect we will want to allow theme() to set at least colour, fill, and alpha, potentially also shape, size and linetype. Font family and size should also be considered but likely should inherit from other text elements set by theme().

I have not yet updated documentation, added a NEWS bullet, or additional tests as all of this will be done further down the line. This branch does successfully build and pass tests. Running through existing package examples was very useful for finding side effects.

Visualizing the changes:

# theme_grey() is still our default and delivers expected defaults
ggplot(mpg, aes(displ, hwy)) + geom_point()

# custom themes can be added to change default colour and fill (other aesthetics may be added in the future)
my_theme <- theme(geom = element_geom(colour = "purple", fill = "darkblue"))
ggplot(mpg, aes(displ, hwy)) + geom_point() + my_theme

ggplot(mpg, aes(displ, hwy)) + geom_col() + my_theme

# theme dark now stands out automatically with lighter fill and colour defaults
ggplot(mtcars, aes(cyl, mpg)) + geom_col() + theme_dark() 

# theme had to be fed to guides to be able to handle discrete legends properly
ggplot(mpg, aes(displ, hwy, shape = drv)) + geom_point() + theme_dark()

# note that the mapping still overrides theme defaults
ggplot(mpg, aes(displ, hwy, colour = drv)) + geom_point() + my_theme

# and params still override all mapped and theme aesthetics
ggplot(mpg, aes(displ, hwy, colour = drv)) + geom_point(colour = "red") + my_theme

Created on 2018-07-10 by the reprex package (v0.2.0).

[clauswilke]

I think this is going to be a great addition to ggplot2. One comment that dovetails with something @baptiste mentioned at some point in a related post: It's unlikely that there's going to be one choice of color that is going to work for all geoms. Usually, the way this is solved in design is via a small palette of choices. E.g., there could be a foreground color, a background/fill color, and a few accent colors. The geom defaults would then pick the appropriate choices from this palette.

As an example, themes in PowerPoint have 10 colors, background 1, background 2, text 1, text 2, and accent 1 through 6. For ggplot, where we often need to fill things, I'd propose to use something like background 1 & 2, text 1 & 2, accent 1 - 3, and fill 1 - 3. And if you wanted to make it even simpler, it could be just background, text, accent, and fill.

[dpseidel]

Thanks @clauswilke - the palette idea would solve the variability issue I mentioned above. I just need to think critically about how to choose and document these palettes.

[baptiste]

Nice, thanks for doing this.

For reference, there could be some interesting ideas to pick from Gadfly

(notably: a number of foreground + background tints, but also the sort of automatic colour pairing that I mentioned in the issue (e.g deriving a nice palette by muting/complementing/... a base colour).

[hadley]

I think we'll need at least colour.accent and fill.accent. I'm not sure how we'd define multiple accents (e.g. 1-3) to make it clear where they are used. The googlesheet should be helpful for pulling these together.

We might want to consider naming them colour, colour1 etc to avoid providing too many semantics.

[baptiste]

Alternatively, maybe vectors? colour[1:3]
It might require something like a length check to minimise confusion, but it would seem more extensible than a series of hard-coded variable names which don't add semantic value.

[hadley]

Making it a vector suggests that there could be arbitrarily many of them. I don't know if that's any better.

[baptiste]

True – not entirely sure either if it's better.
But it does give more freedom for theme designers who might want a pool of N colours to pick from for whatever reason, theme_unicorn().

Mostly, I find names like colour1...colour2 quite non-descriptive and rather unhelpful (what if someone wants to generate such themes programmatically – e.g. with an app/add-in –, or extend their number, etc. -> immediately leads to things like assign()/get()/paste()).

[hadley]

I don't think that's an axis of generalisation that will be helpful. This is about enabling built-in geoms to be themeable, so there is a fixed and known set of possible values.

[baptiste]

Not sure what's limiting this to built-in geoms – couldn't the ggunicorn package want to write

GeomUnicorn$default_aes = aes(horn.colour = theme$geom$colour[13])

?

[hadley]

What I'm trying to say is that while we could make it more flexible, I don't think it is a good idea because it makes the implementation more complex for relatively little gain (since it requires both a custom theme and a custom geom, and shatters the independence of themes and geoms).

[baptiste]

Hmm, I guess we're not seeing it the same way – I'll leave it at what I wrote above, should others want to weigh in.
Personally I see no additional complexity in having the theme$geom element specify a vector of colours[1:n] rather than a fixed set of variables such as colour1...colour2. Built-in geoms as implemented in this PR would simply use theme$geom$colours[1], colours[2]... instead of theme$geom$colour1.
Unless I'm missing something this doesn't break the separation of geoms/themes any more than this whole proposal to inherit default values from the theme: it simply offers a more extensible way to do so beyond the built-in geoms, should the need arise in an extension package (and packages like ggthemes, etc. often do pair a theme, a scale, and a geom, in practice).

Edit: the only interaction I could see being a problem is if a user mixes a built-in theme that defines 3 colours with a geom that requires 4 in its default aes settings. But that doesn't seem technically unsurmountable – maybe set a hard limit of 5 and recycle shorter vectors.

[hadley]

Basic approach looks good.

Let's iterate on how the default aesthetics look, and how they are evaluated.

[hadley]

I think the code related to evaluating aesthetics in the special theme environment should be pulled out into its own function so that we can think about it in isolation, and then test it.

[hadley]

If geoms are themeable, I think we can deprecate this function, so we should just leave as is and in the documentation mark it as internal, and point people towards the new theme system.

[dpseidel]

If we deprecate this, we will presumably need to make all aesthetics themeable so as to not lose the functionality?

[hadley]

Oh yeah. In that case we should pull this out into a function that can be shared with eval_defaults()

[hadley]

When you rewrite these calls with many unnamed arguments, can you please rewrite to add names? (i.e. here use theme = plot$theme)

[hadley]

Why can we no longer inherit from point?

[dpseidel]

Because with the expr() wrapper the default_aes no longer played nice with defaults() function or utils::modifyList() which is used by sf. I expect this can/will be reverted with this round of changes.

[dpseidel]

Actually now that I'm looking at this again, geom_sf can't currently inherit any "themeable" aesthetics because this function (and thus these aesthetics) get evaluated in draw_geom step of layer.r (rather than earlier in the get_defaults step as do all other geoms), so it currently passes an unevaluated expression to grDevices::col2rgb() which predictably gives an error . So this needs some refactoring to be "themeable" - either by evaluating aesthetics at the proper step (rather than passing NULL values as geom_sf does now) or by pulling theme into draw_geom so that the sf aesthetics can be evaluated in the context of the plots theme. It would seem to me that the former is a preferable solution but I expect that it was written this way for a reason that will become obvious as I dig in. I'm fixing your other concerns now and will work on sf in a future commit.

[hadley]

Do we need alpha as a default? People could always set individually with alpha()

[dpseidel]

Presumably not. Currently alpha has to be NA by default to respect RGBA colour specification

[hadley]

What if we used fill and fill_1?

And what if we ditched colour + color in favo(u)r of col?

[clauswilke]

With the aesthetics renaming code that I wrote, we should be able to get away with defining just one spelling of colour (probably the British one). The intent was to internally rename everything to that spelling. It probably already works as is, but worst-case scenario a few more renaming calls would have to be added in some places.

[clauswilke]

I.e., take the argument names of element_geom() and run through this function:

ggplot2/R/aes.r

Line 155 in 4d2ca99

standardise_aes_names <- function(x) {

[clauswilke]

However, this would require a ... argument instead of listing everything explicitly, so maybe not the right approach. In any case, I’d argue against col just because in the rest of ggplot2 col is not used. And I think we also should talk about whether it’s colour.accent1 or colour_accent1. We’re not entirely consistent with points vs underscores, and it would be good to pick a scheme and at least be consistent going forward.

[dpseidel]

So I've implemented most of your suggested changes at least enough so that we can iterate on them. A couple notes:

1. I've removed the expr() wrapper which makes a lot of sense but has some downstream effects, notably, since aes() quotes its arguments and given environments are ignored when evaluating quosures (as they have their own)-- I'm now evaluating default_aes() by passing the plot theme as data to eval_tidy(). This works fine with the slightly magical and long theme$geom$col expression or with the less magical (?) but necessarily (AFAICT) 2 argument function theme_aes("col", theme) that I have implemented for GeomPoint as an example.
   

   ggplot2/R/geom-point.r

   Lines 108 to 118 in 23fdf39

   	GeomPoint <- ggproto("GeomPoint", Geom,
   	required_aes = c("x", "y"),
   	non_missing_aes = c("size", "shape", "colour"),
   	default_aes = aes(
   	shape = 19,
   	colour = theme_aes("col", theme),
   	size = 1.5,
   	fill = NA,
   	alpha = NA,
   	stroke = 0.5
   	),

   
   If I want to be able to pass an environment I have to hack the quosures before evaluation which seems really unconventional or consider adapting the aes() or new_aesthetics() functions to specially handle themeable elements which seems like a potential dangerzone for breaking things I don't want to break. Is there a different approach I am missing? Do you still prefer the function or some variant of it?
2. So far I have left the update_geom_defaults() function and geom_sf() as is (see response to your earlier comment), but those will be fixed in a future commit.
3. I updated to col, col_1, etc. but that discussion is more than warranted and I'm happy to change things again.
4. I'll begin adding tests in the next iteration.

[clauswilke]

How about implementing element_geom() along the following lines?

element_geom <- function(fill = NULL,
                         fill_1 = NULL,
                         colour = NULL,
                         colour_1 = NULL,
                         colour_2 = NULL,
                         ...,
                         inherit.blank = FALSE) {
  extra_aes <- ggplot2:::rename_aes(list(...))
  
  aes_list <- modifyList(
    list(
      fill = fill, fill_1 = fill_1, colour = colour,
      colour_1 = colour_1, colour_2 = colour_2,
      inherit.blank = inherit.blank
    ),
    extra_aes,
    keep.null = TRUE
  )
  
  structure(
    aes_list,
    class = c("element_geom", "element")
  )
}

element_geom()
#> List of 6
#>  $ fill         : NULL
#>  $ fill_1       : NULL
#>  $ colour       : NULL
#>  $ colour_1     : NULL
#>  $ colour_2     : NULL
#>  $ inherit.blank: logi FALSE
#>  - attr(*, "class")= chr [1:2] "element_geom" "element"

element_geom(fill = "red", colour = "blue")
#> List of 6
#>  $ fill         : chr "red"
#>  $ fill_1       : NULL
#>  $ colour       : chr "blue"
#>  $ colour_1     : NULL
#>  $ colour_2     : NULL
#>  $ inherit.blank: logi FALSE
#>  - attr(*, "class")= chr [1:2] "element_geom" "element"

element_geom(fill = "red", color = "blue", color_1 = "black")
#> List of 6
#>  $ fill         : chr "red"
#>  $ fill_1       : NULL
#>  $ colour       : chr "blue"
#>  $ colour_1     : chr "black"
#>  $ colour_2     : NULL
#>  $ inherit.blank: logi FALSE
#>  - attr(*, "class")= chr [1:2] "element_geom" "element"

Created on 2018-08-09 by the reprex package (v0.2.0).

[hadley]

This seems like the right approach to me, but I think I'd embed I do:

from_theme <- function(name) {
  # plus error handling
  theme$geom[[name]]
}

Then data = list(from_theme = from_theme)

[dpseidel]

Sorry for the long delay on my end, getting back to this now. I have just embedded and implemented from_theme() as suggested by @hadley and refactored element_geom() as suggested by @clauswilke. Unfortunately, with vdiffr now running properly, this PR will fail on multiple unrelated visual tests because of the way vdiffr adds it's own theme_test() to all plots without a specified theme. vdiffr::theme_test() does not recognize/specify the new element_geom() and thus serves colour = NULL to scales::alpha() which returns a subscript out of bounds error. All non-visual tests pass. I need to add new tests, visual especially, but won't be able to do this without a workaround or solution to the theme_test() issue. I am still working to refactor sf to handle themeable aesthetics and update documentation, examples, and tests.

[dpseidel]

Hey team, I'm so glad there is still interest in this feature! I continue to think this would be really cool and am sorry I let it lie fallow for so long.

I'm working on reviving this zombie PR now. Currently, it's breaking a lot of seemingly unrelated tests which I will work through shortly, but I would appreciate a fresh review of the general approach and scope since so much has changed internally since @hadley and I first began implementing this. CC: @clauswilke @thomasp85

[clauswilke]

I looked it over and added some comments.

[clauswilke]

I would add arguments color, color_1, color_2 and use those if the colour versions are NULL (or the other way round, doesn't really matter).

[clauswilke]

Is the "geom" argument in el_def() correct? Doesn't this imply that geom inherits from geom?

[dpseidel]

Hi all,

As of these most recent commits, all geoms now respect default aesthetics set by themes 🎉. Currently I’ve only done this for color, fill, and some text aesthetics however, after reviewing and agreeing on the implementation, we should discuss the scope of aesthetics we want to make “themeable”.

Some notes on current implementation:

In order to get this to work for geom_sf (which dynamically chooses defaults not during compute_geom2 like all other geoms but later during draw_geom), I needed to pull them through from plot build into draw_geom and LayerSf so defaults could be evaluated in their current context.

I originally attempted (viewable in a separate theme_geom-sfopt2 branch) to bring sf default evaluation more in line with the behavior of other geoms by providing all possible default configurations (those for point, polygon, line, & collection) as a list to default_aes so they could all be evaluated in the eval_defaults function and then writing some logic to choose the correct defaults dynamically in the use_defaults function. I made this work preliminarily but such a drastic change to structure of the default_aes object broke a number of things (e.g. parameter evaluation) and I ultimately decided that this approach required touching too many disparate parts of the API. I opted for what seemed to me to be the slightly more conservative approach but I would appreciate any and all thoughtful critiques on this implementation (ping @thomasp85, @clauswilke, @hadley)

Some work still to be done

I still need to review/repair GeomSf draw_key function and currently defunct default_aesthetics function and decide where/how/if it’s necessary to reevaluate defaults in this context.

More granular tests should absolutely be written and as I mentioned above, before this PR is merged there needs to be a larger discussion about scope and I suspect some work to implement further themeable defaults.

[clauswilke]

I'm not sure I like the special casing of draw_layer() for GeomSf. We had at some point discussed handing more info about the plot into the draw_layer() function, in the context of making the scales available. I wonder whether both issues could be solved at once. @paleolimbot What was your final thought on getting the scales into draw_layer()?

[paleolimbot]

I believe that is still held up on a decision about layer-specific scales. I don't understand layer-specific scales, so I don't know where it stands. If there are no layer-specific scales, then then I think the layout is reasonable, given that there's no other stateful object to keep track of that kind of thing through the build process.

Would it be worth making the theme available everywhere during the plot build/render? Or is that too confusing? It might allow more flexibility customizing other things. The mechanics of it aren't difficult (see https://github.com/paleolimbot/ggr6/blob/master/R/theme.R#L28-L44 ).

[clauswilke]

My thinking was simply: We now have two very different cases that suggest draw_layer() needs more info that in currently receives. Instead of developing weird workarounds, why not break the signature once and hand it the objects that seem relevant. (And maybe add ... to future-proof the function.)

However, I don't remember the discussion about layer-specific scales. Would handing draw_layer() the layout object interfere with that?

[thomasp85]

I think there are two things being conflated with "layer specific scales" (at least in my head). The discussion I remember entered around allowing the geom access to the scales used for the different aesthetics...

The other thing, which I have thought about a bit recently, is an API for setting scales on a per-layer base, such that two layer could use e.g. two different colour scales

[clauswilke]

Let's move the discussion about the function signature of draw_layer() into a separate issue. I'm going to open one. If we decide to move forward with it, we can quickly merge a PR that makes this change, and then @dpseidel can use it in this PR here.

[clauswilke]

Please use #3854 for the discussion of draw_layer().

[cpsievert]

Have y'all considered that there are a handful of different levels of (grayscale) fill in ggplot2 alone (likely for a reason)?

> geoms <- mget(ls(asNamespace("ggplot2"), pattern = "^Geom[A-Z]"), asNamespace("ggplot2"))
> unique(unlist(lapply(geoms, function(g) g$default_aes$fill)))
[1] "grey20" "grey35" "white"  NA       "black"  "grey50" "grey60"

With that in mind, a more robust (and backwards compatible) interface would be something like theme(geom = element_geom(bg = "white", fg = "black", accent = "#3366FF", ...)) and hold the geom responsible for mixing the background and foreground colors. For example, GeomRect$default_aes$fill currently defaults to "gray35" (i.e., interpolate 35% of the way from fg to bg), so the default could be:

GeomRect$default_aes$fill = scales::colour_ramp(c(from_theme('bg'), from_theme('fg')), alpha = TRUE)(0.35)

By the way, I've been thinking a lot about this sort of stuff recently for the new wip auto-theming feature coming to shiny, and I'm thinking of abstracting out some of that functionality into a self-contained package so it could be used in any context. Towards that end, having this PR, as well as #3828 and #3833, would be hugely helpful in avoiding terrible hacks to reach that goal. Also, by using the interface I propose above, it'd be more consistent with the approach I'll be taking

[baptiste]

In one of my earlier attempts at suggesting this geom-defaults-in-themes feature I floated the idea of extending rel() beyond sizes; one could think of defining a few main colours — a bit like a css-variables model — and have functions to calculate derivative shades (e.g desaturated, complement, lighter/darker, etc.)

(The css analogy isn't entirely unmotivated as it seems likely more and more use-cases will want to style ggplots consistently with a surrounding webpage (cf Joe's comment on Shiny, for instance))

Edit: found one of these earlier discussions: #2173 (comment)

[paleolimbot]

(I implemented something like this in my just-for-fun ggr6 package...you can pass a function as a theme element that transforms the parent!)

[thomasp85]

Closing in favour of #5833