From e3b7822c378c3cf0ff5bdc20b0f4d95a6afbe098 Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Sat, 15 Apr 2023 10:07:34 +0200 Subject: [PATCH] Warn when using constant aesthetics (#5256) * Add constant check * Exclude geom_{ab/h/v}line from check * Add test * Pass constructor as call in warning --- R/geom-abline.R | 4 +++- R/geom-hline.R | 4 +++- R/geom-vline.R | 4 +++- R/layer.R | 9 ++++++++- tests/testthat/_snaps/layer.md | 5 +++++ tests/testthat/test-layer.R | 8 ++++++++ 6 files changed, 30 insertions(+), 4 deletions(-) diff --git a/R/geom-abline.R b/R/geom-abline.R index fa81b366d4..da65483635 100644 --- a/R/geom-abline.R +++ b/R/geom-abline.R @@ -147,5 +147,7 @@ GeomAbline <- ggproto("GeomAbline", Geom, draw_key = draw_key_abline, - rename_size = TRUE + rename_size = TRUE, + + check_constant_aes = FALSE ) diff --git a/R/geom-hline.R b/R/geom-hline.R index eb8c118ff2..924b41f40a 100644 --- a/R/geom-hline.R +++ b/R/geom-hline.R @@ -60,5 +60,7 @@ GeomHline <- ggproto("GeomHline", Geom, draw_key = draw_key_path, - rename_size = TRUE + rename_size = TRUE, + + check_constant_aes = FALSE ) diff --git a/R/geom-vline.R b/R/geom-vline.R index c8c9491f10..2705093f05 100644 --- a/R/geom-vline.R +++ b/R/geom-vline.R @@ -60,5 +60,7 @@ GeomVline <- ggproto("GeomVline", Geom, draw_key = draw_key_vline, - rename_size = TRUE + rename_size = TRUE, + + check_constant_aes = FALSE ) diff --git a/R/layer.R b/R/layer.R index de2b96fc34..737fbff7bd 100644 --- a/R/layer.R +++ b/R/layer.R @@ -292,15 +292,22 @@ Layer <- ggproto("Layer", NULL, } n <- nrow(data) + aes_n <- lengths(evaled) if (n == 0) { # No data, so look at longest evaluated aesthetic if (length(evaled) == 0) { n <- 0 } else { - aes_n <- lengths(evaled) n <- if (min(aes_n) == 0) 0L else max(aes_n) } } + if ((self$geom$check_constant_aes %||% TRUE) + && length(aes_n) > 0 && all(aes_n == 1) && n > 1) { + cli::cli_warn(c( + "All aesthetics have length 1, but the data has {n} rows.", + i = "Did you mean to use {.fn annotate}?" + ), call = self$constructor) + } check_aesthetics(evaled, n) # Set special group and panel vars diff --git a/tests/testthat/_snaps/layer.md b/tests/testthat/_snaps/layer.md index 058d22c4d5..641c3d3dc3 100644 --- a/tests/testthat/_snaps/layer.md +++ b/tests/testthat/_snaps/layer.md @@ -84,6 +84,11 @@ ! Can only draw one boxplot per group i Did you forget `aes(group = ...)`? +# layer warns for constant aesthetics + + All aesthetics have length 1, but the data has 32 rows. + i Did you mean to use `annotate()`? + # layer_data returns a data.frame `layer_data()` must return a diff --git a/tests/testthat/test-layer.R b/tests/testthat/test-layer.R index 3ce1791eac..9bb87b6361 100644 --- a/tests/testthat/test-layer.R +++ b/tests/testthat/test-layer.R @@ -123,6 +123,14 @@ test_that("layer reports the error with correct index etc", { expect_snapshot_error(ggplotGrob(p)) }) +test_that("layer warns for constant aesthetics", { + p <- ggplot(mtcars, aes(x = seq_along(mpg))) + geom_point(aes(y = 2)) + expect_silent(ggplot_build(p)) + + p <- ggplot(mtcars, aes(x = 1)) + geom_point(aes(y = 2)) + expect_snapshot_warning(ggplot_build(p)) +}) + # Data extraction --------------------------------------------------------- test_that("layer_data returns a data.frame", {