Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn on the use of ..var.. and stat(var) #4950

Merged
4 changes: 3 additions & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ Imports:
grid,
gtable (>= 0.1.1),
isoband,
lifecycle,
lifecycle (> 1.0.1),
MASS,
mgcv,
rlang (>= 1.0.0),
Expand Down Expand Up @@ -71,6 +71,8 @@ Suggests:
testthat (>= 3.1.2),
vdiffr (>= 1.0.0),
xml2
Remotes:
r-lib/lifecycle
Enhances:
sp
VignetteBuilder:
Expand Down
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# ggplot2 (development version)

* The dot-dot notation (`..var..`) and `stat()`, which have been superseded by
`after_stat()`, are now formally deprecated (@yutannihilation, #3693).

* `geom_col()` and `geom_bar()` gain a new `just` argument. This is set to `0.5`
by default; use `just = 0`/`just = 1` to place columns on the left/right
of the axis breaks.
Expand Down
25 changes: 19 additions & 6 deletions R/aes-evaluation.r
Original file line number Diff line number Diff line change
Expand Up @@ -93,31 +93,44 @@ is_dotted_var <- function(x) {
}

# Determine if aesthetic is calculated
is_calculated_aes <- function(aesthetics) {
vapply(aesthetics, is_calculated, logical(1), USE.NAMES = FALSE)
is_calculated_aes <- function(aesthetics, warn = FALSE) {
vapply(aesthetics, is_calculated, warn = warn, logical(1), USE.NAMES = FALSE)
}
is_scaled_aes <- function(aesthetics) {
vapply(aesthetics, is_scaled, logical(1), USE.NAMES = FALSE)
}
is_staged_aes <- function(aesthetics) {
vapply(aesthetics, is_staged, logical(1), USE.NAMES = FALSE)
}
is_calculated <- function(x) {
is_calculated <- function(x, warn = FALSE) {
if (is_call(get_expr(x), "after_stat")) {
return(TRUE)
}
# Support of old recursive behaviour
if (is.atomic(x)) {
FALSE
} else if (is.symbol(x)) {
is_dotted_var(as.character(x))
res <- is_dotted_var(as.character(x))
if (res && warn) {
what <- I(glue("The dot-dot notation (`{x}`)"))
var <- gsub(match_calculated_aes, "\\1", as.character(x))
with <- I(glue("`after_stat({var})`"))
lifecycle::deprecate_warn("3.4.0", what, with, id = "ggplot-warn-aes-dot-dot")
}
res
} else if (is_quosure(x)) {
is_calculated(quo_get_expr(x))
is_calculated(quo_get_expr(x), warn = warn)
} else if (is.call(x)) {
if (identical(x[[1]], quote(stat))) {
if (warn) {
what <- I(glue("`{expr_deparse(x)}`"))
x[[1]] <- quote(after_stat)
with <- I(glue("`{expr_deparse(x)}`"))
lifecycle::deprecate_warn("3.4.0", what, with, id = "ggplot-warn-aes-stat")
}
TRUE
} else {
any(vapply(x, is_calculated, logical(1)))
any(vapply(x, is_calculated, warn = warn, logical(1)))
}
} else if (is.pairlist(x)) {
FALSE
Expand Down
20 changes: 4 additions & 16 deletions R/layer.r
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,7 @@ layer <- function(geom = NULL, stat = NULL,
if (geom$rename_size && "size" %in% extra_param && !"linewidth" %in% mapped_aesthetics(mapping)) {
aes_params <- c(aes_params, params["size"])
extra_param <- setdiff(extra_param, "size")
# TODO: move to cli_warn()
cli::cli_inform(c(
"{.field size} aesthetic has been deprecated for use with lines as of ggplot2 3.4.0",
"i" = "Please use {.field linewidth} aesthetic instead"
), .frequency = "regularly", .frequency_id = "ggplot-size-linewidth")
lifecycle::deprecate_warn("3.4.0", I("Using `size` aesthetic for lines"), I("`linewidth`"))
}
if (check.param && length(extra_param) > 0) {
cli::cli_warn("Ignoring unknown parameters: {.arg {extra_param}}", call = call_env)
Expand All @@ -146,11 +142,7 @@ layer <- function(geom = NULL, stat = NULL,
# Take care of size->linewidth aes renaming
if (geom$rename_size && "size" %in% extra_aes && !"linewidth" %in% mapped_aesthetics(mapping)) {
extra_aes <- setdiff(extra_aes, "size")
# TODO: move to cli_warn()
cli::cli_inform(c(
"{.field size} aesthetic has been deprecated for use with lines as of ggplot2 3.4.0",
"i" = "Please use {.field linewidth} aesthetic instead"
), .frequency = "regularly", .frequency_id = "ggplot-size-linewidth")
lifecycle::deprecate_warn("3.4.0", I("Using `size` aesthetic for lines"), I("`linewidth`"))
}
if (check.aes && length(extra_aes) > 0) {
cli::cli_warn("Ignoring unknown aesthetics: {.field {extra_aes}}", call = call_env)
Expand Down Expand Up @@ -247,11 +239,7 @@ Layer <- ggproto("Layer", NULL,
!"linewidth" %in% names(self$computed_mapping) &&
"linewidth" %in% self$geom$aesthetics()) {
self$computed_mapping$size <- plot$mapping$size
# TODO: move to cli_warn()
cli::cli_inform(c(
"{.field size} aesthetic has been deprecated for use with lines as of ggplot2 3.4.0",
"i" = "Please use {.field linewidth} aesthetic instead"
), .frequency = "regularly", .frequency_id = "ggplot-size-linewidth")
lifecycle::deprecate_warn("3.4.0", I("Using `size` aesthetic for lines"), I("`linewidth`"))
}
# defaults() strips class, but it needs to be preserved for now
class(self$computed_mapping) <- "uneval"
Expand All @@ -267,7 +255,7 @@ Layer <- ggproto("Layer", NULL,

# Drop aesthetics that are set or calculated
set <- names(aesthetics) %in% names(self$aes_params)
calculated <- is_calculated_aes(aesthetics)
calculated <- is_calculated_aes(aesthetics, warn = TRUE)
modifiers <- is_scaled_aes(aesthetics)

aesthetics <- aesthetics[!set & !calculated & !modifiers]
Expand Down
10 changes: 10 additions & 0 deletions tests/testthat/_snaps/aes-calculated.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,13 @@

Duplicated aesthetics after name standardisation: colour

# A deprecated warning is issued when stat(var) or ..var.. is used

`stat(foo)` was deprecated in ggplot2 3.4.0.
Please use `after_stat(foo)` instead.

---

The dot-dot notation (`..bar..`) was deprecated in ggplot2 3.4.0.
Please use `after_stat(bar)` instead.

2 changes: 1 addition & 1 deletion tests/testthat/_snaps/layer.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
x `fill = after_stat(data)`
i Did you map your stat in the wrong layer?

# function aesthetics are wrapped with stat()
# function aesthetics are wrapped with after_stat()

Problem while computing aesthetics.
i Error occurred in the 1st layer.
Expand Down
8 changes: 8 additions & 0 deletions tests/testthat/test-aes-calculated.r
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,11 @@ test_that("staged aesthetics warn appropriately for duplicated names", {
# One warning in building due to `stage()`/`after_scale()`
expect_snapshot_warning(ggplot_build(p))
})

test_that("A deprecated warning is issued when stat(var) or ..var.. is used", {
p1 <- ggplot(NULL, aes(stat(foo)))
expect_snapshot_warning(b1 <- ggplot_build(p1))

p2 <- ggplot(NULL, aes(..bar..))
expect_snapshot_warning(b2 <- ggplot_build(p2))
})
8 changes: 4 additions & 4 deletions tests/testthat/test-geom-sf.R
Original file line number Diff line number Diff line change
Expand Up @@ -99,18 +99,18 @@ test_that("geom_sf() removes rows containing missing aes", {
colour = c("red", NA)
)

p <- ggplot(pts) + geom_sf()
p <- ggplot(pts)
expect_warning(
expect_identical(grob_xy_length(p + aes(size = size)), c(1L, 1L)),
expect_identical(grob_xy_length(p + geom_sf(aes(size = size))), c(1L, 1L)),
Comment on lines +102 to +104
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed a tweak here because of this issue: #4960

"Removed 1 row containing missing values"
)
expect_warning(
expect_identical(grob_xy_length(p + aes(shape = shape)), c(1L, 1L)),
expect_identical(grob_xy_length(p + geom_sf(aes(shape = shape))), c(1L, 1L)),
"Removed 1 row containing missing values"
)
# default colour scale maps a colour even to a NA, so identity scale is needed to see if NA is removed
expect_warning(
expect_identical(grob_xy_length(p + aes(colour = colour) + scale_colour_identity()),
expect_identical(grob_xy_length(p + geom_sf(aes(colour = colour)) + scale_colour_identity()),
c(1L, 1L)),
"Removed 1 row containing missing values"
)
Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/test-layer.r
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ test_that("missing aesthetics trigger informative error", {
)
})

test_that("function aesthetics are wrapped with stat()", {
test_that("function aesthetics are wrapped with after_stat()", {
df <- data_frame(x = 1:10)
expect_snapshot_error(
ggplot_build(ggplot(df, aes(colour = density, fill = density)) + geom_point())
Expand All @@ -65,7 +65,7 @@ test_that("function aesthetics are wrapped with stat()", {
test_that("computed stats are in appropriate layer", {
df <- data_frame(x = 1:10)
expect_snapshot_error(
ggplot_build(ggplot(df, aes(colour = stat(density), fill = stat(density))) + geom_point())
ggplot_build(ggplot(df, aes(colour = after_stat(density), fill = after_stat(density))) + geom_point())
)
})

Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-stat-contour.R
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ test_that("geom_contour() and stat_contour() result in identical layer data", {

test_that("basic stat_contour() plot builds", {
p <- ggplot(faithfuld, aes(waiting, eruptions)) +
geom_contour(aes(z = density, col = factor(stat(level))))
geom_contour(aes(z = density, col = factor(after_stat(level))))

# stat_contour() visual tests are unstable due to the
# implementation in isoband
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-stat-sf-coordinates.R
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ test_that("stat_sf_coordinates() retrieves coordinates from sf objects", {
# computed variables (x and y)
df_point <- sf::st_sf(geometry = sf::st_sfc(sf::st_point(c(1, 2))))
expect_identical(
comp_sf_coord(df_point, aes(x = stat(x) + 10, y = stat(y) * 10))[, c("x", "y")],
comp_sf_coord(df_point, aes(x = after_stat(x) + 10, y = after_stat(y) * 10))[, c("x", "y")],
data_frame(x = 11, y = 20)
)
})
Expand Down