From 654e247bb0dd672658d40e027b16e133e0a3d540 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Wed, 27 Nov 2024 11:53:53 +0100 Subject: [PATCH 1/5] exempt some coords from transforming x/y AsIs variables --- R/coord-polar.R | 4 ++++ R/coord-radial.R | 4 ++++ R/coord-sf.R | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/R/coord-polar.R b/R/coord-polar.R index f1c8108ddf..fbaa336682 100644 --- a/R/coord-polar.R +++ b/R/coord-polar.R @@ -180,6 +180,10 @@ CoordPolar <- ggproto("CoordPolar", Coord, }, transform = function(self, data, panel_params) { + if (inherits(data$x, "AsIs") && inherits(data$y, "AsIs")) { + return(data) + } + arc <- self$start + c(0, 2 * pi) dir <- self$direction data <- rename_data(self, data) diff --git a/R/coord-radial.R b/R/coord-radial.R index 3a5ccf1ee2..c385832ec6 100644 --- a/R/coord-radial.R +++ b/R/coord-radial.R @@ -275,6 +275,10 @@ CoordRadial <- ggproto("CoordRadial", Coord, }, transform = function(self, data, panel_params) { + if (inherits(data$x, "AsIs") && inherits(data$y, "AsIs")) { + return(data) + } + data <- rename_data(self, data) bbox <- panel_params$bbox %||% list(x = c(0, 1), y = c(0, 1)) arc <- panel_params$arc %||% c(0, 2 * pi) diff --git a/R/coord-sf.R b/R/coord-sf.R index 45cc7a90fb..92a5a12da5 100644 --- a/R/coord-sf.R +++ b/R/coord-sf.R @@ -77,6 +77,10 @@ CoordSf <- ggproto("CoordSf", CoordCartesian, }, transform = function(self, data, panel_params) { + if (inherits(data$x, "AsIs") && inherits(data$y, "AsIs")) { + return(data) + } + # we need to transform all non-sf data into the correct coordinate system source_crs <- panel_params$default_crs target_crs <- panel_params$crs From 238553f9c6c40dc5f764d15066e3fbdfbd9eaff2 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Wed, 27 Nov 2024 11:54:07 +0100 Subject: [PATCH 2/5] add tests --- tests/testthat/test-coord-polar.R | 16 ++++++++++++++++ tests/testthat/test-coord_sf.R | 16 ++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/tests/testthat/test-coord-polar.R b/tests/testthat/test-coord-polar.R index da49368108..e1bf70c655 100644 --- a/tests/testthat/test-coord-polar.R +++ b/tests/testthat/test-coord-polar.R @@ -155,6 +155,22 @@ test_that("bounding box calculations are sensible", { ) }) +test_that("when both x and y are AsIs, they are not transformed", { + + p <- ggplot() + + annotate("text", x = I(0.75), y = I(0.25), label = "foo") + + scale_x_continuous(limits = c(0, 10)) + + scale_y_continuous(limits = c(0, 10)) + + grob <- get_layer_grob(p + coord_polar())[[1]] + location <- c(as.numeric(grob$x), as.numeric(grob$y)) + expect_equal(location, c(0.75, 0.25)) + + grob <- get_layer_grob(p + coord_radial())[[1]] + location <- c(as.numeric(grob$x), as.numeric(grob$y)) + expect_equal(location, c(0.75, 0.25)) + +}) # Visual tests ------------------------------------------------------------ diff --git a/tests/testthat/test-coord_sf.R b/tests/testthat/test-coord_sf.R index a25d470e3b..70cdbb9d20 100644 --- a/tests/testthat/test-coord_sf.R +++ b/tests/testthat/test-coord_sf.R @@ -314,6 +314,22 @@ test_that("sf_transform_xy() works", { }) +test_that("when both x and y are AsIs, they are not transformed", { + + skip_if_not_installed("sf") + + p <- ggplot() + + annotate("text", x = I(0.75), y = I(0.25), label = "foo") + + scale_x_continuous(limits = c(-180, 180)) + + scale_y_continuous(limits = c(-80, 80)) + + coord_sf(default_crs = 4326, crs = 3857) + + grob <- get_layer_grob(p)[[1]] + location <- c(as.numeric(grob$x), as.numeric(grob$y)) + expect_equal(location, c(0.75, 0.25)) + +}) + test_that("coord_sf() can use function breaks and n.breaks", { polygon <- sf::st_sfc( From c65a491078a8ccf6cb5fa2c8c0ef6046c88f3b68 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Wed, 27 Nov 2024 11:56:31 +0100 Subject: [PATCH 3/5] add news bullet --- NEWS.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/NEWS.md b/NEWS.md index 0c493a8f58..1767b39489 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,8 @@ # ggplot2 (development version) +* In non-orthogonal coordinate systems (`coord_sf()`, `coord_polar()` and + `coord_radial()`), using 'AsIs' variables escape transformation when + both `x` and `y` is an 'AsIs' variable (@teunbrand, #6205). * Custom and raster annotation now respond to scale transformations, and can use AsIs variables for relative placement (@teunbrand based on @yutannihilation's prior work, #3120) From b1d45c843ad43f0855ebc407ab62f0a72732e1f6 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Wed, 27 Nov 2024 13:01:34 +0100 Subject: [PATCH 4/5] throw warning when only one of two variables is AsIs --- R/coord-.R | 21 +++++++++++++++++++++ R/coord-polar.R | 2 +- R/coord-radial.R | 2 +- R/coord-sf.R | 2 +- tests/testthat/_snaps/coord-polar.md | 4 ++++ tests/testthat/test-coord-polar.R | 9 +++++++++ 6 files changed, 37 insertions(+), 3 deletions(-) diff --git a/R/coord-.R b/R/coord-.R index 5e711cf95c..6aa113d3ff 100644 --- a/R/coord-.R +++ b/R/coord-.R @@ -284,3 +284,24 @@ check_coord_limits <- function( check_object(limits, is_vector, "a vector", arg = arg, call = call) check_length(limits, 2L, arg = arg, call = call) } + +is_transform_immune <- function(data, coord_name) { + x <- inherits(data$x, "AsIs") + y <- inherits(data$y, "AsIs") + if (!(x || y)) { + # Neither variable is AsIs, so we need to transform + return(FALSE) + } + if (x && y) { + # Both variables are AsIs, so no need to transform + return(TRUE) + } + # We're now in the `xor(x, y)` case + var <- if (x) "x" else "y" + alt <- if (x) "y" else "x" + cli::cli_warn( + "{.fn {coord_name}} cannot respect the {.cls AsIs} class of {.var {var}} \\ + when {.var {alt}} is not also {.cls AsIs}." + ) + return(FALSE) +} diff --git a/R/coord-polar.R b/R/coord-polar.R index fbaa336682..aa5e7c538b 100644 --- a/R/coord-polar.R +++ b/R/coord-polar.R @@ -180,7 +180,7 @@ CoordPolar <- ggproto("CoordPolar", Coord, }, transform = function(self, data, panel_params) { - if (inherits(data$x, "AsIs") && inherits(data$y, "AsIs")) { + if (is_transform_immune(data, snake_class(self))) { return(data) } diff --git a/R/coord-radial.R b/R/coord-radial.R index c385832ec6..b028822f0e 100644 --- a/R/coord-radial.R +++ b/R/coord-radial.R @@ -275,7 +275,7 @@ CoordRadial <- ggproto("CoordRadial", Coord, }, transform = function(self, data, panel_params) { - if (inherits(data$x, "AsIs") && inherits(data$y, "AsIs")) { + if (is_transform_immune(data, snake_class(self))) { return(data) } diff --git a/R/coord-sf.R b/R/coord-sf.R index 92a5a12da5..2a43d2e5ef 100644 --- a/R/coord-sf.R +++ b/R/coord-sf.R @@ -77,7 +77,7 @@ CoordSf <- ggproto("CoordSf", CoordCartesian, }, transform = function(self, data, panel_params) { - if (inherits(data$x, "AsIs") && inherits(data$y, "AsIs")) { + if (is_transform_immune(data, snake_class(self))) { return(data) } diff --git a/tests/testthat/_snaps/coord-polar.md b/tests/testthat/_snaps/coord-polar.md index 1e43119dbe..92d1f0afca 100644 --- a/tests/testthat/_snaps/coord-polar.md +++ b/tests/testthat/_snaps/coord-polar.md @@ -12,3 +12,7 @@ No appropriate placement found for `r_axis_inside`. i Axis will be placed at panel edge. +# when both x and y are AsIs, they are not transformed + + `coord_name()` cannot respect the class of `x` when `y` is not also . + diff --git a/tests/testthat/test-coord-polar.R b/tests/testthat/test-coord-polar.R index e1bf70c655..27b641f964 100644 --- a/tests/testthat/test-coord-polar.R +++ b/tests/testthat/test-coord-polar.R @@ -170,6 +170,15 @@ test_that("when both x and y are AsIs, they are not transformed", { location <- c(as.numeric(grob$x), as.numeric(grob$y)) expect_equal(location, c(0.75, 0.25)) + # Check warning is thrown if only one is AsIs + p <- ggplot() + + annotate("text", x = I(0.75), y = 2.5, label = "foo") + + scale_x_continuous(limits = c(0, 10)) + + scale_y_continuous(limits = c(0, 10)) + + coord_radial() + + expect_snapshot_warning(ggplotGrob(p)) + }) # Visual tests ------------------------------------------------------------ From 462fe426876fc1ac48d89b73e0a176416045ee48 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Wed, 27 Nov 2024 13:20:48 +0100 Subject: [PATCH 5/5] update snapshot --- tests/testthat/_snaps/coord-polar.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/_snaps/coord-polar.md b/tests/testthat/_snaps/coord-polar.md index 92d1f0afca..62138ccf99 100644 --- a/tests/testthat/_snaps/coord-polar.md +++ b/tests/testthat/_snaps/coord-polar.md @@ -14,5 +14,5 @@ # when both x and y are AsIs, they are not transformed - `coord_name()` cannot respect the class of `x` when `y` is not also . + `coord_radial()` cannot respect the class of `x` when `y` is not also .