Skip to content

Commit

Permalink
Warn when using constant aesthetics (#5256)
Browse files Browse the repository at this point in the history
* Add constant check

* Exclude geom_{ab/h/v}line from check

* Add test

* Pass constructor as call in warning
  • Loading branch information
teunbrand authored Apr 15, 2023
1 parent 66462be commit e3b7822
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 4 deletions.
4 changes: 3 additions & 1 deletion R/geom-abline.R
Original file line number Diff line number Diff line change
Expand Up @@ -147,5 +147,7 @@ GeomAbline <- ggproto("GeomAbline", Geom,

draw_key = draw_key_abline,

rename_size = TRUE
rename_size = TRUE,

check_constant_aes = FALSE
)
4 changes: 3 additions & 1 deletion R/geom-hline.R
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,7 @@ GeomHline <- ggproto("GeomHline", Geom,

draw_key = draw_key_path,

rename_size = TRUE
rename_size = TRUE,

check_constant_aes = FALSE
)
4 changes: 3 additions & 1 deletion R/geom-vline.R
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,7 @@ GeomVline <- ggproto("GeomVline", Geom,

draw_key = draw_key_vline,

rename_size = TRUE
rename_size = TRUE,

check_constant_aes = FALSE
)
9 changes: 8 additions & 1 deletion R/layer.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions tests/testthat/_snaps/layer.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <data.frame>
Expand Down
8 changes: 8 additions & 0 deletions tests/testthat/test-layer.R
Original file line number Diff line number Diff line change
Expand Up @@ -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", {
Expand Down

0 comments on commit e3b7822

Please sign in to comment.