Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Layers have names #5967

Merged
merged 10 commits into from
Aug 26, 2024
Merged

Layers have names #5967

merged 10 commits into from
Aug 26, 2024

Conversation

teunbrand
Copy link
Collaborator

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

Copy link
Member

@thomasp85 thomasp85 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@teunbrand teunbrand changed the title POC: named layers Layers have names Aug 26, 2024
Merge branch 'main' into named_layers

# Conflicts:
#	R/plot-construction.R
@teunbrand teunbrand merged commit 5482939 into tidyverse:main Aug 26, 2024
11 checks passed
@teunbrand teunbrand deleted the named_layers branch August 26, 2024 11:19
@yutannihilation
Copy link
Member

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
Copy link
Collaborator Author

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add layer names
3 participants