From 3d4c518d9fb48d0c995e7c300c41e72b6b414a11 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 1 Jul 2024 17:01:54 +0200 Subject: [PATCH 1/8] use `%||%` for `na.rm` --- R/layer.R | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/R/layer.R b/R/layer.R index 8b4621bde2..5d3d4aac7a 100644 --- a/R/layer.R +++ b/R/layer.R @@ -128,9 +128,6 @@ layer <- function(geom = NULL, stat = NULL, position <- check_subclass(position, "Position", env = parent.frame(), call = call_env) # Special case for na.rm parameter needed by all layers - if (is.null(params$na.rm)) { - params$na.rm <- FALSE - } # Special case for key_glyph parameter which is handed in through # params since all geoms/stats forward ... to params @@ -138,6 +135,7 @@ layer <- function(geom = NULL, stat = NULL, key_glyph <- params$key_glyph params$key_glyph <- NULL # remove to avoid warning about unknown parameter } + params$na.rm <- params$na.rm %||% FALSE # Split up params between aesthetics, geom, and stat params <- rename_aes(params) From aa30217f10a9c80c77a58f727e24c883cc0ca01d Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 1 Jul 2024 17:03:02 +0200 Subject: [PATCH 2/8] simplify special `key_glyph` case --- R/layer.R | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/R/layer.R b/R/layer.R index 5d3d4aac7a..c60ef3a1f5 100644 --- a/R/layer.R +++ b/R/layer.R @@ -128,13 +128,6 @@ layer <- function(geom = NULL, stat = NULL, position <- check_subclass(position, "Position", env = parent.frame(), call = call_env) # Special case for na.rm parameter needed by all layers - - # Special case for key_glyph parameter which is handed in through - # params since all geoms/stats forward ... to params - if (!is.null(params$key_glyph)) { - key_glyph <- params$key_glyph - params$key_glyph <- NULL # remove to avoid warning about unknown parameter - } params$na.rm <- params$na.rm %||% FALSE # Split up params between aesthetics, geom, and stat @@ -143,7 +136,8 @@ layer <- function(geom = NULL, stat = NULL, geom_params <- params[intersect(names(params), geom$parameters(TRUE))] stat_params <- params[intersect(names(params), stat$parameters(TRUE))] - all <- c(geom$parameters(TRUE), stat$parameters(TRUE), geom$aesthetics()) + ignore <- "key_glyph" + all <- c(geom$parameters(TRUE), stat$parameters(TRUE), geom$aesthetics(), ignore) # Take care of plain patterns provided as aesthetic pattern <- vapply(aes_params, is_pattern, logical(1)) @@ -177,7 +171,7 @@ layer <- function(geom = NULL, stat = NULL, } # adjust the legend draw key if requested - geom <- set_draw_key(geom, key_glyph) + geom <- set_draw_key(geom, key_glyph %||% params$key_glyph) fr_call <- layer_class$constructor %||% frame_call(call_env) From 942c01663d3a7428bc8bb1decba9c505201c9cf0 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 1 Jul 2024 17:03:22 +0200 Subject: [PATCH 3/8] add `name` field to LayerInstance objects --- R/layer.R | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/R/layer.R b/R/layer.R index c60ef3a1f5..825c271785 100644 --- a/R/layer.R +++ b/R/layer.R @@ -136,7 +136,7 @@ layer <- function(geom = NULL, stat = NULL, geom_params <- params[intersect(names(params), geom$parameters(TRUE))] stat_params <- params[intersect(names(params), stat$parameters(TRUE))] - ignore <- "key_glyph" + ignore <- c("key_glyph", "name") all <- c(geom$parameters(TRUE), stat$parameters(TRUE), geom$aesthetics(), ignore) # Take care of plain patterns provided as aesthetic @@ -186,7 +186,8 @@ layer <- function(geom = NULL, stat = NULL, aes_params = aes_params, position = position, inherit.aes = inherit.aes, - show.legend = show.legend + show.legend = show.legend, + name = params$name ) } From 6396864035fbd870b154428c8976dc93d96608b9 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 1 Jul 2024 17:10:20 +0200 Subject: [PATCH 4/8] helper for layer names --- R/plot-construction.R | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/R/plot-construction.R b/R/plot-construction.R index b6d83fe1f0..8c7bd63ffc 100644 --- a/R/plot-construction.R +++ b/R/plot-construction.R @@ -183,3 +183,20 @@ ggplot_add.Layer <- function(object, plot, object_name) { } plot } + +new_layer_names <- function(layer, existing) { + new_name <- layer$name + if (is.null(new_name)) { + # Construct a name from the layer's call + new_name <- call_name(layer$constructor) + + if (new_name %in% existing) { + names <- c(existing, new_name) + names <- vec_as_names(names, repair = "unique", quiet = TRUE) + new_name <- names[length(names)] + } + } + + names <- c(existing, new_name) + vec_as_names(names, repair = "check_unique") +} From 71fa72cd98ccb064116849ee6741df0f2694a253 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 1 Jul 2024 17:10:38 +0200 Subject: [PATCH 5/8] apply layer names --- R/plot-construction.R | 2 ++ 1 file changed, 2 insertions(+) diff --git a/R/plot-construction.R b/R/plot-construction.R index 8c7bd63ffc..1ac30fdbbb 100644 --- a/R/plot-construction.R +++ b/R/plot-construction.R @@ -166,7 +166,9 @@ ggplot_add.by <- function(object, plot, object_name) { #' @export ggplot_add.Layer <- function(object, plot, object_name) { + layers_names <- new_layer_names(object, names(plot$layers)) plot$layers <- append(plot$layers, object) + names(plot$layers) <- layers_names # Add any new labels mapping <- make_labels(object$mapping) From 54491094f4ffde5be45d78a548fbeb85b66eed1e Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 1 Jul 2024 17:13:54 +0200 Subject: [PATCH 6/8] add tests --- tests/testthat/test-layer.R | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/testthat/test-layer.R b/tests/testthat/test-layer.R index 0a89af1da8..c7cd3b1bb7 100644 --- a/tests/testthat/test-layer.R +++ b/tests/testthat/test-layer.R @@ -138,6 +138,22 @@ test_that("layer warns for constant aesthetics", { expect_snapshot_warning(ggplot_build(p)) }) +test_that("layer names can be resolved", { + + p <- ggplot() + geom_point() + geom_point() + expect_equal(names(p$layers), c("geom_point", "geom_point...2")) + + p <- ggplot() + geom_point(name = "foo") + geom_point(name = "bar") + expect_equal(names(p$layers), c("foo", "bar")) + + l <- geom_point(name = "foobar") + expect_error( + p + l + l, + "names are duplicated" + ) +}) + + # Data extraction --------------------------------------------------------- test_that("AsIs data passes unmodified", { From 650e9a69dea910c32ccfa27a9912fa63cc245a2c Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 1 Jul 2024 17:18:27 +0200 Subject: [PATCH 7/8] add bullet --- NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.md b/NEWS.md index e99235a3a0..c475e55248 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ # ggplot2 (development version) +* Layers can have names (@teunbrand, #4066). * (internal) The plot's layout now has a coord parameter that is used to prevent setting up identical panel parameters (#5427) * (internal) rearranged the code of `Facet$draw_panels()` method (@teunbrand). From 78fa04908ea5acc5fb68f6d5301babd72ab49303 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 2 Jul 2024 13:57:12 +0200 Subject: [PATCH 8/8] fallback for direct `layer()` calls --- R/layer.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/layer.R b/R/layer.R index 825c271785..ca5fcd5174 100644 --- a/R/layer.R +++ b/R/layer.R @@ -173,7 +173,7 @@ layer <- function(geom = NULL, stat = NULL, # adjust the legend draw key if requested geom <- set_draw_key(geom, key_glyph %||% params$key_glyph) - fr_call <- layer_class$constructor %||% frame_call(call_env) + fr_call <- layer_class$constructor %||% frame_call(call_env) %||% current_call() ggproto("LayerInstance", layer_class, constructor = fr_call,