From 4fafb96c99036a184b6835dfee7e0b1bf925522b Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 7 Apr 2025 15:21:41 +0200 Subject: [PATCH 1/5] deprecate `optional_aes` fields --- R/geom-.R | 15 +++++++++++---- R/stat-.R | 15 +++++++++++---- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/R/geom-.R b/R/geom-.R index db3dd2d5cc..c5c045150f 100644 --- a/R/geom-.R +++ b/R/geom-.R @@ -222,12 +222,19 @@ Geom <- ggproto("Geom", }, aesthetics = function(self) { - if (is.null(self$required_aes)) { - required_aes <- NULL - } else { + required_aes <- self$required_aes + if (!is.null(required_aes)) { required_aes <- unlist(strsplit(self$required_aes, '|', fixed = TRUE)) } - c(union(required_aes, names(self$default_aes)), self$optional_aes, "group") + optional_aes <- self$optional_aes + if (length(optional_aes) > 1) { + deprecate_soft0( + "4.0.0", + I("The `optional_aes` field"), + I("NULL-aesthetics in `default_aes`") + ) + } + c(union(required_aes, names(self$default_aes)), optional_aes, "group") }, # Should the geom rename size to linewidth? diff --git a/R/stat-.R b/R/stat-.R index 11cdbc67d6..afefae8c58 100644 --- a/R/stat-.R +++ b/R/stat-.R @@ -209,12 +209,19 @@ Stat <- ggproto("Stat", }, aesthetics = function(self) { - if (is.null(self$required_aes)) { - required_aes <- NULL - } else { + required_aes <- self$required_aes + if (!is.null(self$required_aes)) { required_aes <- unlist(strsplit(self$required_aes, '|', fixed = TRUE)) } - c(union(required_aes, names(self$default_aes)), self$optional_aes, "group") + optional_aes <- self$optional_aes + if (length(optional_aes) > 1) { + deprecate_soft0( + "4.0.0", + I("The `optional_aes` field"), + I("NULL-aesthetics in `default_aes`") + ) + } + c(union(required_aes, names(self$default_aes)), optional_aes, "group") } ) From 58200dde69734b88810ba89c4923fa5669c0e3bb Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 7 Apr 2025 15:24:42 +0200 Subject: [PATCH 2/5] express optional aesthetics as NULL-aesthetics in `default_aes` --- R/geom-rug.R | 3 +-- R/geom-smooth.R | 2 +- R/stat-ellipse.R | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/R/geom-rug.R b/R/geom-rug.R index bcc7adca7a..f469f4a880 100644 --- a/R/geom-rug.R +++ b/R/geom-rug.R @@ -86,7 +86,6 @@ geom_rug <- function(mapping = NULL, data = NULL, #' @usage NULL #' @export GeomRug <- ggproto("GeomRug", Geom, - optional_aes = c("x", "y"), draw_panel = function(self, data, panel_params, coord, lineend = "butt", sides = "bl", outside = FALSE, length = unit(0.03, "npc")) { @@ -153,7 +152,7 @@ GeomRug <- ggproto("GeomRug", Geom, gTree(children = inject(gList(!!!rugs))) }, - default_aes = GeomPath$default_aes, + default_aes = aes(x = NULL, y = NULL, !!!GeomPath$default_aes), draw_key = draw_key_path, diff --git a/R/geom-smooth.R b/R/geom-smooth.R index c386504fa8..c860da1c9c 100644 --- a/R/geom-smooth.R +++ b/R/geom-smooth.R @@ -166,9 +166,9 @@ GeomSmooth <- ggproto("GeomSmooth", Geom, draw_key = draw_key_smooth, required_aes = c("x", "y"), - optional_aes = c("ymin", "ymax"), default_aes = aes( + ymin = NULL, ymax = NULL, colour = from_theme(colour %||% accent), fill = from_theme(fill %||% col_mix(ink, paper, 0.6)), linewidth = from_theme(2 * linewidth), diff --git a/R/stat-ellipse.R b/R/stat-ellipse.R index d462bf3575..04521b40d3 100644 --- a/R/stat-ellipse.R +++ b/R/stat-ellipse.R @@ -77,7 +77,7 @@ stat_ellipse <- function(mapping = NULL, data = NULL, #' @export StatEllipse <- ggproto("StatEllipse", Stat, required_aes = c("x", "y"), - optional_aes = "weight", + default_aes = aes(weight = NULL), dropped_aes = "weight", setup_params = function(data, params) { From 3e9e2aef14b36cc9fe440cf68ad8f8a7afa76039 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 7 Apr 2025 15:27:44 +0200 Subject: [PATCH 3/5] add news bullet --- NEWS.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/NEWS.md b/NEWS.md index 363a20cc28..1d71a4086f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,8 @@ # ggplot2 (development version) +* The `optional_aes` field in `Geom` and `Stat` classes is now deprecated in + favour of using `default_aes = aes(foo = NULL)` to mark the 'foo' aesthetic + as optional (@teunbrand, #6393). * `position_fill()` avoids stacking observations of zero (@teunbrand, #6338) * New `layer(layout)` argument to interact with facets (@teunbrand, #3062) * New `stat_connect()` to connect points via steps or other shapes From a7665da9fca42e51998b75d0efd0179e82e41203 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 7 Apr 2025 15:51:46 +0200 Subject: [PATCH 4/5] fix mistake --- R/geom-.R | 2 +- R/stat-.R | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/R/geom-.R b/R/geom-.R index c5c045150f..4a501e4ef3 100644 --- a/R/geom-.R +++ b/R/geom-.R @@ -227,7 +227,7 @@ Geom <- ggproto("Geom", required_aes <- unlist(strsplit(self$required_aes, '|', fixed = TRUE)) } optional_aes <- self$optional_aes - if (length(optional_aes) > 1) { + if (length(optional_aes) > 0) { deprecate_soft0( "4.0.0", I("The `optional_aes` field"), diff --git a/R/stat-.R b/R/stat-.R index afefae8c58..518681e474 100644 --- a/R/stat-.R +++ b/R/stat-.R @@ -214,7 +214,7 @@ Stat <- ggproto("Stat", required_aes <- unlist(strsplit(self$required_aes, '|', fixed = TRUE)) } optional_aes <- self$optional_aes - if (length(optional_aes) > 1) { + if (length(optional_aes) > 0) { deprecate_soft0( "4.0.0", I("The `optional_aes` field"), From 266e4b69e6b368a812cb8292cd6d3bd30d5d4157 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 7 Apr 2025 15:52:00 +0200 Subject: [PATCH 5/5] add test --- tests/testthat/test-geom-.R | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/testthat/test-geom-.R b/tests/testthat/test-geom-.R index 6766178f22..0aada68985 100644 --- a/tests/testthat/test-geom-.R +++ b/tests/testthat/test-geom-.R @@ -64,9 +64,6 @@ test_that("updating geom aesthetic defaults preserves class and order", { }) - - - test_that("updating stat aesthetic defaults preserves class and order", { original_defaults <- StatBin$default_aes @@ -86,3 +83,8 @@ test_that("updating stat aesthetic defaults preserves class and order", { update_stat_defaults("bin", NULL) }) + +test_that("Geom throws lifecycle signals", { + test <- ggproto(NULL, GeomPoint, optional_aes = "foo") + lifecycle::expect_deprecated(stat_identity(geom = test)) +})