From 7746c221ef738100b8a52e0db59b25c29288a419 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 11 Nov 2025 16:48:03 +0100 Subject: [PATCH 1/4] mapped aesthetics as variable --- R/layer.R | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/R/layer.R b/R/layer.R index 179f519441..74c2e50d79 100644 --- a/R/layer.R +++ b/R/layer.R @@ -118,6 +118,7 @@ layer <- function(geom = NULL, stat = NULL, if (!is.null(mapping)) { mapping <- validate_mapping(mapping, call_env) } + mapped_aes <- mapped_aesthetics(mapping) data <- fortify(data) @@ -146,7 +147,7 @@ layer <- function(geom = NULL, stat = NULL, # Warn about extra params and aesthetics extra_param <- setdiff(names(params), all) # Take care of size->linewidth renaming in layer params - if (geom$rename_size && "size" %in% extra_param && !"linewidth" %in% mapped_aesthetics(mapping)) { + if (geom$rename_size && "size" %in% extra_param && !"linewidth" %in% mapped_aes) { aes_params <- c(aes_params, params["size"]) extra_param <- setdiff(extra_param, "size") deprecate_warn0("3.4.0", I("Using `size` aesthetic for lines"), I("`linewidth`"), user_env = user_env) @@ -156,11 +157,11 @@ layer <- function(geom = NULL, stat = NULL, } extra_aes <- setdiff( - mapped_aesthetics(mapping), + mapped_aes, c(geom$aesthetics(), stat$aesthetics(), position$aesthetics()) ) # Take care of size->linewidth aes renaming - if (geom$rename_size && "size" %in% extra_aes && !"linewidth" %in% mapped_aesthetics(mapping)) { + if (geom$rename_size && "size" %in% extra_aes && !"linewidth" %in% mapped_aes) { extra_aes <- setdiff(extra_aes, "size") deprecate_warn0("3.4.0", I("Using `size` aesthetic for lines"), I("`linewidth`"), user_env = user_env) } From bac92d7c319630336cf933ba489d0e2b3d3897af Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 11 Nov 2025 16:51:13 +0100 Subject: [PATCH 2/4] all aesthetics as variable --- R/layer.R | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/R/layer.R b/R/layer.R index 74c2e50d79..5c91ad045b 100644 --- a/R/layer.R +++ b/R/layer.R @@ -130,13 +130,14 @@ layer <- function(geom = NULL, stat = NULL, params$na.rm <- params$na.rm %||% FALSE # Split up params between aesthetics, geom, and stat + all_aes <- unique(c(geom$aesthetics(), position$aesthetics(), stat$aesthetics())) params <- rename_aes(params) - aes_params <- params[intersect(names(params), union(geom$aesthetics(), position$aesthetics()))] + aes_params <- params[intersect(names(params), all_aes)] geom_params <- params[intersect(names(params), geom$parameters(TRUE))] stat_params <- params[intersect(names(params), stat$parameters(TRUE))] ignore <- c("key_glyph", "name", "layout") - all <- c(geom$parameters(TRUE), stat$parameters(TRUE), geom$aesthetics(), position$aesthetics(), ignore) + all <- c(geom$parameters(TRUE), stat$parameters(TRUE), all_aes, ignore) # Take care of plain patterns provided as aesthetic pattern <- vapply(aes_params, is_pattern, logical(1)) @@ -156,10 +157,7 @@ layer <- function(geom = NULL, stat = NULL, cli::cli_warn("Ignoring unknown parameters: {.arg {extra_param}}", call = call_env) } - extra_aes <- setdiff( - mapped_aes, - c(geom$aesthetics(), stat$aesthetics(), position$aesthetics()) - ) + extra_aes <- setdiff(mapped_aes, all_aes) # Take care of size->linewidth aes renaming if (geom$rename_size && "size" %in% extra_aes && !"linewidth" %in% mapped_aes) { extra_aes <- setdiff(extra_aes, "size") From 56e6d3130f16617a25e214cf620cac421cce4537 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Wed, 12 Nov 2025 10:31:43 +0100 Subject: [PATCH 3/4] add warning --- R/layer.R | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/R/layer.R b/R/layer.R index 5c91ad045b..38f18bca20 100644 --- a/R/layer.R +++ b/R/layer.R @@ -153,8 +153,21 @@ layer <- function(geom = NULL, stat = NULL, extra_param <- setdiff(extra_param, "size") deprecate_warn0("3.4.0", I("Using `size` aesthetic for lines"), I("`linewidth`"), user_env = user_env) } - if (check.param && length(extra_param) > 0) { - cli::cli_warn("Ignoring unknown parameters: {.arg {extra_param}}", call = call_env) + if (check.param) { + if (length(extra_param) > 0) { + cli::cli_warn("Ignoring unknown parameters: {.arg {extra_param}}", call = call_env) + } + double_defined <- intersect(mapped_aes, names(aes_params)) + if (length(double_defined) > 0) { + cli::cli_warn( + c( + "The {.and {.field {double_defined}}} aesthetic{?s} {?is/are} \\ + defined twice: once in {.arg mapping} and once as a static aesthetic.", + "i" = "The static aesthetic overrules the mapped aesthetic." + ), + call = call_env + ) + } } extra_aes <- setdiff(mapped_aes, all_aes) From b5f582b4b0ffd94026424f7cc17dcd4811d9d080 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Wed, 12 Nov 2025 10:34:05 +0100 Subject: [PATCH 4/4] add test --- tests/testthat/_snaps/layer.md | 5 +++++ tests/testthat/test-layer.R | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/tests/testthat/_snaps/layer.md b/tests/testthat/_snaps/layer.md index a7ae1d1a85..babc3b3fd7 100644 --- a/tests/testthat/_snaps/layer.md +++ b/tests/testthat/_snaps/layer.md @@ -47,6 +47,11 @@ Ignoring empty aesthetics: `fill` and `shape`. +# aesthetics defined twice create warning + + The size aesthetic is defined twice: once in `mapping` and once as a static aesthetic. + i The static aesthetic overrules the mapped aesthetic. + # invalid aesthetics throws errors Problem while computing aesthetics. diff --git a/tests/testthat/test-layer.R b/tests/testthat/test-layer.R index 9528c2927f..3dd0f08852 100644 --- a/tests/testthat/test-layer.R +++ b/tests/testthat/test-layer.R @@ -33,6 +33,10 @@ test_that("empty aesthetics create warning", { expect_snapshot_warning(ggplot_build(p)) }) +test_that("aesthetics defined twice create warning", { + expect_snapshot_warning(geom_point(aes(size = foo), size = 12)) +}) + test_that("invalid aesthetics throws errors", { # We want to test error and ignore the scale search message suppressMessages({