Default pointshape of 19 can cause confusion with fill guides

[arcresu]

Occasionally it can be useful to use filled shapes (i.e. in the range 21:25) to separately control the colour and fill aesthetics in geom_point(). Here's a contrived example using shape 21 that works:

library(ggplot2)
#> 
#> Attaching package: 'ggplot2'
#> The following object is masked from 'package:base':
#> 
#>     is.element

ggplot(head(diamonds, n = 20)) +
  geom_point(aes(carat, price, fill = cut), shape = 21, size = 5)

Now if we also map the shape aesthetic, the fill guide draws the keys using its default shape of 19 which doesn't support fill leading to the colours disappearing from the legend:

ggplot(head(diamonds, n = 20)) +
  geom_point(aes(carat, price, fill = cut, shape = color), size = 5) +
  scale_shape_manual(values = 21:25)

It can of course be fixed by overriding the shape to be 21 as in the first example:

ggplot(head(diamonds, n = 20)) +
  geom_point(aes(carat, price, fill = cut, shape = color), size = 5) +
  scale_fill_ordinal(guide = guide_legend(override.aes = list(shape = 21))) +
  scale_shape_manual(values = 21:25)

^{Created on 2024-11-06 with reprex v2.1.1}

This isn't really a bug because it's behaving as intended, but it's still a bit surprising! Some possible solutions:

1. Warn when constructing/drawing a guide that has key_glyph = "point", a non-empty/non-constant fill, and an unfilled shape.
2. Note the behaviour somewhere in documentation.
3. Do nothing.

Note that even though draw_key_point() fills in a default of 19 that doesn't seem relevant in practice because the key is always draw with a non-NULL value determined by the theme's pointshape if shape isn't mapped for a guide.

Aside: using a key glyph outside shape scale

Even putting aside filled shapes, if my shape scale is only triangles and squares, should any of the guides really be using circles for the keys? I suspect that by now people are used to thinking of circles as the "neutral" point shape in ggplot.

library(ggplot2)
#> 
#> Attaching package: 'ggplot2'
#> The following object is masked from 'package:base':
#> 
#>     is.element

ggplot(head(diamonds, n = 20)) +
  geom_point(aes(carat, price, shape = cut, colour = color), size = 5) +
  scale_shape_manual(values = 2:6)

^{Created on 2024-11-06 with reprex v2.1.1}

[arcresu]

Here's a minimal implementation of the warning that I don't love since it's very noisy:

697002311 (HEAD -> warn_unfilled_point) [3 minutes ago, Carl Suster]
  Warn if point key incompatible with fill

diff --git a/R/geom-point.R b/R/geom-point.R
index 3efa394c3..51a285cd8 100644
--- a/R/geom-point.R
+++ b/R/geom-point.R
@@ -236,3 +236,11 @@ translate_shape_string <- function(shape_string) {
 
   unname(pch_table[shape_match])
 }
+
+is_filled_shape <- function(shape) {
+  if (is.character(shape)) {
+    shape <- translate_shape_string(shape)
+  }
+
+  shape %in% 21:25
+}
diff --git a/R/guide-legend.R b/R/guide-legend.R
index 37aad2e3f..37f267fe0 100644
--- a/R/guide-legend.R
+++ b/R/guide-legend.R
@@ -237,6 +237,7 @@ GuideLegend <- ggproto(
       matched_aes <- matched_aes(layer, params)
       key <- params$key[matched_aes]
       key$.id <- seq_len(nrow(key))
+      key$.aesthetics <- list(matched_aes)
 
       # Filter static aesthetics to those with single values
       single_params <- lengths(layer$aes_params) == 1L
diff --git a/R/legend-draw.R b/R/legend-draw.R
index ccfb03587..aced22a7d 100644
--- a/R/legend-draw.R
+++ b/R/legend-draw.R
@@ -24,6 +24,14 @@ NULL
 #' @export
 #' @rdname draw_key
 draw_key_point <- function(data, params, size) {
+  fill_wanted <- mapply(\(a) "fill" %in% a, data$.aesthetics)
+  if (fill_wanted && !is_filled_shape(data$shape)) {
+    cli::cli_warn(c(
+      "Drawing guide for fill with unfilled shape {data$shape}",
+      "i" = "Consider overriding the key shape for the fill scale like {.code guides(fill = guide_legend(override.aes = list(shape = 'circle filled')))}"
+    ))
+  }
+
   if (is.null(data$shape)) {
     data$shape <- 19
   } else if (is.character(data$shape)) {

It adds data$.aesthetics as a list column for the draw_key_*() functions so they know which aesthetics they're supposed to be demonstrating and can complain if the grob they use won't support them. Technically they can already work out the aesthetics by the order of columns in data since the matched aesthetics are the first columns, followed by .id, then the values computed by the stat/geom.

The warning might only apply to some keys (e.g. when the fill guide includes both filled and unfilled shapes), so it has to be checked for each key. The duplicate warnings are excessive though so it would be better to group them and/or have a way to suppress the warning altogether.

In action

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

ggplot(head(diamonds, n = 20)) +
  geom_point(aes(carat, price, fill = cut, shape = color, size = cut)) +
  scale_shape_manual(values = 21:25)
#> Warning: Drawing guide for fill with unfilled shape 19
#> ℹ Consider overriding the key shape for the fill scale like `guides(fill =
#>   guide_legend(override.aes = list(shape = 'circle filled')))`
#> Drawing guide for fill with unfilled shape 19
#> ℹ Consider overriding the key shape for the fill scale like `guides(fill =
#>   guide_legend(override.aes = list(shape = 'circle filled')))`
#> Drawing guide for fill with unfilled shape 19
#> ℹ Consider overriding the key shape for the fill scale like `guides(fill =
#>   guide_legend(override.aes = list(shape = 'circle filled')))`
#> Drawing guide for fill with unfilled shape 19
#> ℹ Consider overriding the key shape for the fill scale like `guides(fill =
#>   guide_legend(override.aes = list(shape = 'circle filled')))`
#> Drawing guide for fill with unfilled shape 19
#> ℹ Consider overriding the key shape for the fill scale like `guides(fill =
#>   guide_legend(override.aes = list(shape = 'circle filled')))`

ggplot(head(diamonds, n = 20)) +
  geom_point(aes(carat, price, fill = cut, shape = color, size = cut)) +
  guides(fill = guide_legend(override.aes = list(shape = 'circle filled'))) +
  scale_shape_manual(values = 21:25)

^{Created on 2024-11-06 with reprex v2.1.1}

[teunbrand]

Hi there thanks for the report!

We appreciate the time you put in building reproducible examples and exploring possible solutions.
This issue was orginally reported in #4049 and the decision rendered then was as follows (#4049 (comment)):

When people venture into shapes 21-25 they're accepting that they'll have to deal with minor inconveniences such as this one. It's an advanced use case that can lead to unintuitive results on occasion.

Regarding some of the specifics:

Note that even though draw_key_point() fills in a default of 19 that doesn't seem relevant in practice because the key is always draw with a non-NULL value determined by the theme's pointshape if shape isn't mapped for a guide.

This is true for geom_point() but not when using draw_key_point() in other geoms.

Even putting aside filled shapes, if my shape scale is only triangles and squares, should any of the guides really be using circles for the keys

The fill scale should be ignorant of other scales, so yes, it should ignore the actual values of your shape scale. It should obey shape when provided as fixed aesthetic, or be dynamic when the fill and shape scales treat the same values.

Here's a minimal implementation

Ideally we don't want to hardcode around the fill aesthetic as people can in theory declare synonymous aesthetics in some niche cases.

[arcresu]

Ah sorry I missed the earlier report. Please feel free to close this if your stance hasn't changed :)

My motivation is just that I ran into this scenario while helping someone and it took me quite a while to work out what was going on even though in hindsight it's clear why it behaves that way. It's really surprising to see a guide where the key can't represent the aesthetic that it's supposed to be explaining. A warning could help someone else discover the solution faster.

Anecdotally, filled points are less "advanced" than might be expected because there are code snippets out there advocating for them (e.g. here, here, here). I've seen people who know about filled shapes but are less familiar with how guides work, so aren't able to debug the guide so fall back on fixing colour in Illustrator or similar.

This is true for geom_point() but not when using draw_key_point() in other geoms.

Good point. I only mentioned this because my first attempt was to override the default shape in draw_key_point() but that had no effect since geom_point() applies the theme default first.

The fill scale should be ignorant of other scales, so yes, it should ignore the actual values of your shape scale.

I agree that makes sense as a ggplot axiom. In specific cases it might not be the best result, but that's what override.aes is for.

Ideally we don't want to hardcode around the fill aesthetic as people can in theory declare synonymous aesthetics in some niche cases.

I thought that by this stage we were close enough to the grid code that matched_aes only contained actual drawing params rather than aesthetics? Or do you mean just the wording of the warning?

Initially I thought this might be an instance of a more generic problem of guides not being able to represent their intended aesthetic, but on reflection it might just be confined to geom_point() with shape 21 and a fill (or synonymous aesthetic) guide. Shapes 22-24 technically have the same issue, but since they're visually different to shape 19 in the absence of fill, it's pretty clear what to do to fix it. Would you be open to a small note in the documentation for geom_point() and/or the specs vignette in lieu of a warning?

[teunbrand]

I thought that by this stage we were close enough to the grid code that matched_aes only contained actual drawing params rather than aesthetics? Or do you mean just the wording of the warning?

Extension packages can use variants of fill for their aesthetic that serves the same role but isn't named fill for various reasons. These reasons can be they want different scales for their fill synonym than the global fill or incorporate multiple types of fill within one geom.

Would you be open to a small note in the documentation for geom_point() and/or the specs vignette in lieu of a warning?

Yes I think this would be the right to do. Having an example for geom_point() with guide_legend(override.aes) would be nice.