From 8606347ef3de59c9aeed845f3fbcaf33c6e0cb20 Mon Sep 17 00:00:00 2001 From: laurabrianna Date: Thu, 15 Aug 2024 13:07:41 -0700 Subject: [PATCH 1/6] Fixes #3533 - helpful error msg on date vs datetime scale --- R/scale-date.R | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/R/scale-date.R b/R/scale-date.R index 36f8b37d83..921e254bf0 100644 --- a/R/scale-date.R +++ b/R/scale-date.R @@ -369,6 +369,10 @@ ScaleContinuousDatetime <- ggproto("ScaleContinuousDatetime", ScaleContinuous, i = "The value was converted to {obj_type_friendly(x)}." ), call = self$call) } + if (inherits(x, "Date")) { + cli::cli_abort("You supplied a {.cls {class(x)}} field. + Did you mean to use {.fn scale_*_date}?", call = self$call) + } ggproto_parent(ScaleContinuous, self)$transform(x) }, map = function(self, x, limits = self$get_limits()) { @@ -416,6 +420,10 @@ ScaleContinuousDate <- ggproto("ScaleContinuousDate", ScaleContinuous, i = "The value was converted to {obj_type_friendly(x)}." ), call = self$call) } + if (inherits(x, "POSIXct")) { + cli::cli_abort("You supplied a {.cls {class(x)}} field. + Did you mean to use {.fn scale_*_datetime}?", call = self$call) + } ggproto_parent(ScaleContinuous, self)$transform(x) }, get_breaks = function(self, limits = self$get_limits()) { From 63d4bda7026c76f9920122045c47a634d4dd3894 Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Mon, 11 Nov 2024 11:47:19 +0100 Subject: [PATCH 2/6] Apply suggestions from code review Co-authored-by: Thomas Lin Pedersen --- R/scale-date.R | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/R/scale-date.R b/R/scale-date.R index 921e254bf0..12cddcba04 100644 --- a/R/scale-date.R +++ b/R/scale-date.R @@ -370,8 +370,7 @@ ScaleContinuousDatetime <- ggproto("ScaleContinuousDatetime", ScaleContinuous, ), call = self$call) } if (inherits(x, "Date")) { - cli::cli_abort("You supplied a {.cls {class(x)}} field. - Did you mean to use {.fn scale_*_date}?", call = self$call) + x <- as.POSIXct(x) } ggproto_parent(ScaleContinuous, self)$transform(x) }, @@ -421,8 +420,7 @@ ScaleContinuousDate <- ggproto("ScaleContinuousDate", ScaleContinuous, ), call = self$call) } if (inherits(x, "POSIXct")) { - cli::cli_abort("You supplied a {.cls {class(x)}} field. - Did you mean to use {.fn scale_*_datetime}?", call = self$call) + x <- as.Date(x) } ggproto_parent(ScaleContinuous, self)$transform(x) }, From e5c9c008e9d1cb0ceb3bd9185c86a0e7c7c4d23b Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Mon, 11 Nov 2024 11:47:19 +0100 Subject: [PATCH 3/6] Apply suggestions from code review Co-authored-by: Thomas Lin Pedersen --- R/scale-date.R | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/R/scale-date.R b/R/scale-date.R index 921e254bf0..b06a49e3d3 100644 --- a/R/scale-date.R +++ b/R/scale-date.R @@ -370,8 +370,7 @@ ScaleContinuousDatetime <- ggproto("ScaleContinuousDatetime", ScaleContinuous, ), call = self$call) } if (inherits(x, "Date")) { - cli::cli_abort("You supplied a {.cls {class(x)}} field. - Did you mean to use {.fn scale_*_date}?", call = self$call) + x <- as.POSIXct(x) } ggproto_parent(ScaleContinuous, self)$transform(x) }, @@ -421,8 +420,7 @@ ScaleContinuousDate <- ggproto("ScaleContinuousDate", ScaleContinuous, ), call = self$call) } if (inherits(x, "POSIXct")) { - cli::cli_abort("You supplied a {.cls {class(x)}} field. - Did you mean to use {.fn scale_*_datetime}?", call = self$call) + x <- as.Date(x) } ggproto_parent(ScaleContinuous, self)$transform(x) }, From 6b680a423fc903f630ec1d6596d0c3b2eaae4e73 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 11 Nov 2024 11:52:21 +0100 Subject: [PATCH 4/6] add news bullet --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index 9d08380189..e73ad3cb51 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # ggplot2 (development version) +* Date scales silently coerce to and datetime scales silently + coerce to (@laurabrianna, #3533) * `geom_contour()` should be able to recognise a rotated grid of points (@teunbrand, #4320) * `geom_boxplot()` gains additional arguments to style the colour, linetype and From c5ee8b8c37a0420c5e3682bda928722f41db06ba Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 11 Nov 2024 11:58:53 +0100 Subject: [PATCH 5/6] add test --- tests/testthat/test-scale_date.R | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/testthat/test-scale_date.R b/tests/testthat/test-scale_date.R index f12a35716c..6ca8e0dcc5 100644 --- a/tests/testthat/test-scale_date.R +++ b/tests/testthat/test-scale_date.R @@ -1,4 +1,19 @@ +test_that("date(time) scales coerce data types", { + + date <- as.Date("2024-11-11") + datetime <- as.POSIXct(date) + + sc <- scale_x_datetime() + df <- sc$transform_df(data.frame(x = date)) + expect_equal(df$x, as.numeric(datetime)) + + sc <- scale_x_date() + df <- sc$transform_df(data.frame(x = datetime)) + expect_equal(df$x, as.numeric(date)) + +}) + # Visual tests ------------------------------------------------------------ test_that("date scale draws correctly", { From fbf82f679a27419e91c34cf01d557ceb8176d3bb Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 11 Nov 2024 12:01:12 +0100 Subject: [PATCH 6/6] use internal data.frame --- tests/testthat/test-scale_date.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-scale_date.R b/tests/testthat/test-scale_date.R index 6ca8e0dcc5..b9a788bb70 100644 --- a/tests/testthat/test-scale_date.R +++ b/tests/testthat/test-scale_date.R @@ -5,11 +5,11 @@ test_that("date(time) scales coerce data types", { datetime <- as.POSIXct(date) sc <- scale_x_datetime() - df <- sc$transform_df(data.frame(x = date)) + df <- sc$transform_df(data_frame0(x = date)) expect_equal(df$x, as.numeric(datetime)) sc <- scale_x_date() - df <- sc$transform_df(data.frame(x = datetime)) + df <- sc$transform_df(data_frame0(x = datetime)) expect_equal(df$x, as.numeric(date)) })