tidyverse / ggplot2

An implementation of the Grammar of Graphics in R
https://ggplot2.tidyverse.org
Other
6.52k stars 2.02k forks source link

Implement smart merging of guides so inappropriate key glyphs are omitted #3648

Closed clauswilke closed 11 months ago

clauswilke commented 4 years ago

Currently, if two layers have non-overlapping values in an aesthetic and different key glyphs, both glyphs are drawn for all data values, as seen here:

library(ggplot2)

ggplot(mtcars, aes(disp, mpg)) + 
  geom_point(aes(color = "points")) +
  geom_smooth(aes(color = "regression line"))
#> `geom_smooth()` using method = 'loess' and formula 'y ~ x'

This is suboptimal, because the line glyph is not sensible for the "points" entry and the point glyph is not sensible for the "regression line" entry. The workaround currently is to use override.aes(), but it is cumbersome and requires deep knowledge about the default settings for various key glyphs:


ggplot(mtcars, aes(disp, mpg)) + 
  geom_point(aes(color = "points")) +
  geom_smooth(aes(color = "regression line")) +
  guides(
    color = guide_legend(
      override.aes = list(
        shape = c(19, NA),
        linetype = c(NA, 1),
        fill = c(NA, "grey60")
      )
    )
  )
#> `geom_smooth()` using method = 'loess' and formula 'y ~ x'

There should be an option, and maybe made the default, to produce the second outcome whenever appropriate.

paleolimbot commented 4 years ago

For reference, I think this is the relevant location:

https://github.com/tidyverse/ggplot2/blob/64248088462cc3114d0eb4e30beafd2550d19284/R/guide-legend.r#L244-L247

I think for this to work, guide$geoms would have to be a list-column in guide$key, so that each item in the guide would have its own set of layer information?

clauswilke commented 4 years ago

I don't have an implementation strategy at this time, I just suspect that it's somehow possible.

yutannihilation commented 4 years ago

Thanks for filing the issue. It seems my comment on https://github.com/tidyverse/ggplot2/pull/3646#issuecomment-559824394 went in the wrong direction, sorry... This looks easier than I thought (though I have no idea the actual implementation would be).

microly commented 4 years ago

I draft a pr (#3649) and try to fix this issue.

microly commented 4 years ago

My pr is the first step and is not smart enough. In the next step, the default value of show.key can set to waiver(), its value can be constructed during the legend building process. Then, this solution will be smart enough.

teunbrand commented 2 years ago

I don't think there currently is a way for the guides to extract some information on whether a layer uses a particular value of an aesthetic. The layers, as presented to the guides, don't know about their data. The scales don't keep track of a layerwise-aesthetic mapping.

It might be possible to do this if either the scale keeps track over what layer has what values, or the layer keeps track over what values for a particular aesthetic were used. However, this would possibly introduce an extra stateful thing in the system, so I don't know if this is wise.

teunbrand commented 2 years ago

After thinking about this for a bit, I think it should be possible to pass along the layer data to the guide_geom() method and filter at that point. From a user's perspective, I think it makes most sense to have show.legend = NA (which is often the default) mean to use the 'smart' labelling. The TRUE and FALSE options can then be used to always and never show the legend key respectively.

Maintainers, do you think it is OK to expose the layer data to the guides? If so, I think I can take this along in the guides to ggproto conversion.

thomasp85 commented 2 years ago

It seems like a pragmatic solution, but maybe pass it into the training part of the guide instead to compartmentalise it a bit?