tidyverse / ggplot2

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

Layers have names #5967

Closed teunbrand closed 1 month ago

teunbrand commented 3 months ago

This PR aims to fix #4066.

Briefly, it sets names for the layers part of a plot that should be able to uniquely identify a layer. They're specified in the params argument to layer() and default to NULL.

As an example of using named layers, we see here that the names provided to the layer end up as the names for the list of layers.

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

p <- ggplot(mpg, aes(displ, hwy)) +
  geom_point(name = "foo") +
  geom_point(name = "bar")

names(p$layers)
#> [1] "foo" "bar"

By default, name = NULL, in which case a name is chosen for a layer. This defaults to the constructor function and that is not unique, it gets a suffix through vec_as_names().

p <- ggplot(mpg, aes(displ, hwy)) +
  geom_point() +
  geom_point()

names(p$layers)
#> [1] "geom_point"     "geom_point...2"

Currently, you cannot set duplicated names. I was unsure whether the correct coarse here is to make names unique or throw an error. I decided it should throw an error on a whim, but I'd be happy to change this.

ggplot(mpg, aes(displ, hwy)) +
  geom_point(name = "foo") +
  geom_point(name = "foo")
#> Error in `new_layer_names()` at ggplot2/R/plot-construction.R:169:3:
#> ! Names must be unique.
#> ✖ These names are duplicated:
#>   * "foo" at locations 1 and 2.

A quick demo that the names can be used to 'zap' layers.

p <- ggplot(mpg, aes(displ, hwy)) +
  geom_point(colour = "blue", alpha = 0.5) +
  geom_point(colour = "red",  alpha = 0.5)

p$layers[["geom_point...2"]] <- NULL
p

Created on 2024-07-01 with reprex v2.1.0

yutannihilation commented 1 month ago

Currently, you cannot set duplicated names. I was unsure whether the correct coarse here is to make names unique or throw an error. I decided it should throw an error on a whim, but I'd be happy to change this.

We might see a few revdep failures on those packages that clone layers. I can fix my package easily, so this isn't a big problem to me, though. Just for sharing.

teunbrand commented 1 month ago

Thanks for the heads up! Do you think it makes more sense to throw a warning instead of an error, and automatically make the names unique?

yutannihilation commented 1 month ago

Thanks. I think making it an error is a good idea. Imagining I would be a user of this feature, I think I would want to assume the names are unique. Let's revisit here if we find difficulty to enforce this change to the revdep packages.