From 122df1d061745f9d1caef2956e294ed17183cc6b Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 12 Mar 2024 15:39:48 +0100 Subject: [PATCH 1/9] Discrete position scales use palette --- R/scale-discrete-.R | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/R/scale-discrete-.R b/R/scale-discrete-.R index 1a8228e318..1fc7773b40 100644 --- a/R/scale-discrete-.R +++ b/R/scale-discrete-.R @@ -134,7 +134,21 @@ ScaleDiscretePosition <- ggproto("ScaleDiscretePosition", ScaleDiscrete, map = function(self, x, limits = self$get_limits()) { if (is.discrete(x)) { - x <- seq_along(limits)[match(as.character(x), limits)] + values <- self$palette(limits) + if (!is.numeric(values)) { + cli::cli_abort( + "The {.arg palette} function must return a {.cls numeric} vector.", + call = self$call + ) + } + if (length(values) < length(limits)) { + cli::cli_abort( + "The {.arg palette} function must return at least \\ + {length(limits)} values.", + call = self$call + ) + } + x <- values[match(as.character(x), limits)] } mapped_discrete(x) }, From 0fd6f37d38cfad91c2af0e8221ff08c86772a70c Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 12 Mar 2024 16:00:22 +0100 Subject: [PATCH 2/9] Plumbing for `palette` argument --- R/scale-discrete-.R | 12 ++++++++---- man/scale_discrete.Rd | 8 +++++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/R/scale-discrete-.R b/R/scale-discrete-.R index 1fc7773b40..8f36c22c00 100644 --- a/R/scale-discrete-.R +++ b/R/scale-discrete-.R @@ -12,6 +12,8 @@ #' #' @inheritDotParams discrete_scale #' @inheritParams discrete_scale +#' @param palette A function that takes the limits as input and provides +#' numerical values as output. #' @rdname scale_discrete #' @family position scales #' @seealso @@ -63,11 +65,12 @@ #' geom_point() + #' scale_x_discrete(labels = abbreviate) #' } -scale_x_discrete <- function(name = waiver(), ..., expand = waiver(), +scale_x_discrete <- function(name = waiver(), ..., palette = seq_along, + expand = waiver(), guide = waiver(), position = "bottom") { sc <- discrete_scale( aesthetics = c("x", "xmin", "xmax", "xend"), name = name, - palette = identity, ..., + palette = palette, ..., expand = expand, guide = guide, position = position, super = ScaleDiscretePosition ) @@ -77,11 +80,12 @@ scale_x_discrete <- function(name = waiver(), ..., expand = waiver(), } #' @rdname scale_discrete #' @export -scale_y_discrete <- function(name = waiver(), ..., expand = waiver(), +scale_y_discrete <- function(name = waiver(), ..., palette = seq_along, + expand = waiver(), guide = waiver(), position = "left") { sc <- discrete_scale( aesthetics = c("y", "ymin", "ymax", "yend"), name = name, - palette = identity, ..., + palette = palette, ..., expand = expand, guide = guide, position = position, super = ScaleDiscretePosition ) diff --git a/man/scale_discrete.Rd b/man/scale_discrete.Rd index 3955316322..4cd85a4fd6 100644 --- a/man/scale_discrete.Rd +++ b/man/scale_discrete.Rd @@ -8,6 +8,7 @@ scale_x_discrete( name = waiver(), ..., + palette = seq_along, expand = waiver(), guide = waiver(), position = "bottom" @@ -16,6 +17,7 @@ scale_x_discrete( scale_y_discrete( name = waiver(), ..., + palette = seq_along, expand = waiver(), guide = waiver(), position = "left" @@ -30,9 +32,6 @@ omitted.} \item{...}{ Arguments passed on to \code{\link[=discrete_scale]{discrete_scale}} \describe{ - \item{\code{palette}}{A palette function that when called with a single integer -argument (the number of levels in the scale) returns the values that -they should take (e.g., \code{\link[scales:pal_hue]{scales::pal_hue()}}).} \item{\code{breaks}}{One of: \itemize{ \item \code{NULL} for no breaks @@ -78,6 +77,9 @@ notation. \item{\code{super}}{The super class to use for the constructed scale} }} +\item{palette}{A function that takes the limits as input and provides +numerical values as output.} + \item{expand}{For position scales, a vector of range expansion constants used to add some padding around the data to ensure that they are placed some distance away from the axes. Use the convenience function \code{\link[=expansion]{expansion()}} From 74bfb9358bec6a7a8dfb37676c7f2c8017cc7a85 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 12 Mar 2024 16:01:10 +0100 Subject: [PATCH 3/9] discrete range derived from mapped limits --- R/coord-transform.R | 2 +- R/scale-expansion.R | 19 +++++++++++-------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/R/coord-transform.R b/R/coord-transform.R index 81f06afad3..83ffd7b9ee 100644 --- a/R/coord-transform.R +++ b/R/coord-transform.R @@ -198,7 +198,7 @@ view_scales_from_scale_with_coord_trans <- function(scale, coord_limits, trans, if (scale$is_discrete()) { continuous_ranges <- expand_limits_discrete_trans( - scale_limits, + scale$map(scale_limits), expansion, coord_limits, trans, diff --git a/R/scale-expansion.R b/R/scale-expansion.R index 9ede4c1400..8ec72c2a78 100644 --- a/R/scale-expansion.R +++ b/R/scale-expansion.R @@ -137,7 +137,7 @@ expand_limits_scale <- function(scale, expand = expansion(0, 0), limits = waiver if (scale$is_discrete()) { coord_limits <- coord_limits %||% c(NA_real_, NA_real_) expand_limits_discrete( - limits, + scale$map(limits), expand, coord_limits, range_continuous = scale$range_c$range @@ -201,14 +201,17 @@ expand_limits_continuous_trans <- function(limits, expand = expansion(0, 0), expand_limits_discrete_trans <- function(limits, expand = expansion(0, 0), coord_limits = c(NA, NA), trans = transform_identity(), range_continuous = NULL) { - if (is.discrete(limits)) { - n_discrete_limits <- length(limits) - } else { - n_discrete_limits <- 0 + discrete_limits <- NULL + if (length(limits) > 0) { + if (is.discrete(limits)) { + discrete_limits <- c(1, length(limits)) # for backward compatibility + } else { + discrete_limits <- range(limits) + } } is_empty <- is.null(limits) && is.null(range_continuous) - is_only_continuous <- n_discrete_limits == 0 + is_only_continuous <- is.null(discrete_limits) is_only_discrete <- is.null(range_continuous) if (is_empty) { @@ -216,10 +219,10 @@ expand_limits_discrete_trans <- function(limits, expand = expansion(0, 0), } else if (is_only_continuous) { expand_limits_continuous_trans(range_continuous, expand, coord_limits, trans) } else if (is_only_discrete) { - expand_limits_continuous_trans(c(1, n_discrete_limits), expand, coord_limits, trans) + expand_limits_continuous_trans(discrete_limits, expand, coord_limits, trans) } else { # continuous and discrete - limit_info_discrete <- expand_limits_continuous_trans(c(1, n_discrete_limits), expand, coord_limits, trans) + limit_info_discrete <- expand_limits_continuous_trans(discrete_limits, expand, coord_limits, trans) # don't expand continuous range if there is also a discrete range limit_info_continuous <- expand_limits_continuous_trans( From 47ce0ba0ed6a7a8c7aa038aa275c4757322ac7f7 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 12 Mar 2024 16:01:22 +0100 Subject: [PATCH 4/9] adjust test --- tests/testthat/test-scale-expansion.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/test-scale-expansion.R b/tests/testthat/test-scale-expansion.R index 7d1b5b30ae..26d25abfdc 100644 --- a/tests/testthat/test-scale-expansion.R +++ b/tests/testthat/test-scale-expansion.R @@ -116,6 +116,6 @@ test_that("expand_limits_scale_discrete() begrudgingly handles numeric limits", coord_limits = c(NA, NA), range_continuous = c(-15, -2) ), - c(-15, -2) + c(-16, -1) ) }) From d74f39daf25a0f8245945679001d73464b9b94b4 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 12 Mar 2024 16:16:28 +0100 Subject: [PATCH 5/9] add tests --- tests/testthat/test-scale-discrete.R | 42 ++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/testthat/test-scale-discrete.R b/tests/testthat/test-scale-discrete.R index d9ce98c494..cd8c2da933 100644 --- a/tests/testthat/test-scale-discrete.R +++ b/tests/testthat/test-scale-discrete.R @@ -162,3 +162,45 @@ test_that("mapped_discrete vectors behaves as predicted", { x[5:7] <- mapped_discrete(seq_len(3)) expect_s3_class(x, "mapped_discrete") }) + +# Palettes ---------------------------------------------------------------- + +test_that("palettes work for discrete scales", { + + df <- data.frame(x = c("A", "B", "C"), y = 1:3) + values <- c(1, 10, 100) + + p <- ggplot(df, aes(x, y)) + + geom_point() + + scale_x_discrete(palette = function(x) values) + + # Check limits are translated to correct values + ld <- layer_data(p) + expect_equal(ld$x, values, ignore_attr = TRUE) + + # Check discsrete expansion is applied + b <- ggplot_build(p) + expect_equal( + b$layout$panel_params[[1]]$x.range, + range(values) + c(-0.6, 0.6) + ) +}) + +test_that("invalid palettes trigger errors", { + + df <- data.frame(x = c("A", "B", "C"), y = 1:3) + + p <- ggplot(df, aes(x, y)) + + geom_point() + + expect_error( + ggplot_build(p + scale_x_discrete(palette = function(x) LETTERS[1:3])), + "must return a .+ vector\\." + ) + + expect_error( + ggplot_build(p + scale_x_discrete(palette = function(x) 1:2)), + "must return at least 3 values" + ) +}) + From 9e0e25d4239fc268b63949058228fc331862bc5c Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Fri, 22 Mar 2024 09:53:29 +0100 Subject: [PATCH 6/9] palettes take `n` just like other scales --- R/scale-discrete-.R | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/R/scale-discrete-.R b/R/scale-discrete-.R index 8f36c22c00..d6dfdd7284 100644 --- a/R/scale-discrete-.R +++ b/R/scale-discrete-.R @@ -65,7 +65,7 @@ #' geom_point() + #' scale_x_discrete(labels = abbreviate) #' } -scale_x_discrete <- function(name = waiver(), ..., palette = seq_along, +scale_x_discrete <- function(name = waiver(), ..., palette = seq_len, expand = waiver(), guide = waiver(), position = "bottom") { sc <- discrete_scale( @@ -80,7 +80,7 @@ scale_x_discrete <- function(name = waiver(), ..., palette = seq_along, } #' @rdname scale_discrete #' @export -scale_y_discrete <- function(name = waiver(), ..., palette = seq_along, +scale_y_discrete <- function(name = waiver(), ..., palette = seq_len, expand = waiver(), guide = waiver(), position = "left") { sc <- discrete_scale( @@ -138,7 +138,7 @@ ScaleDiscretePosition <- ggproto("ScaleDiscretePosition", ScaleDiscrete, map = function(self, x, limits = self$get_limits()) { if (is.discrete(x)) { - values <- self$palette(limits) + values <- self$palette(length(limits)) if (!is.numeric(values)) { cli::cli_abort( "The {.arg palette} function must return a {.cls numeric} vector.", From cf3b28a136ea9a12b59e09984c68fc2d7cd5f1fd Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Thu, 4 Apr 2024 09:48:12 +0200 Subject: [PATCH 7/9] reoxygenate --- man/scale_discrete.Rd | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/man/scale_discrete.Rd b/man/scale_discrete.Rd index 4cd85a4fd6..02359d1f67 100644 --- a/man/scale_discrete.Rd +++ b/man/scale_discrete.Rd @@ -8,7 +8,7 @@ scale_x_discrete( name = waiver(), ..., - palette = seq_along, + palette = seq_len, expand = waiver(), guide = waiver(), position = "bottom" @@ -17,7 +17,7 @@ scale_x_discrete( scale_y_discrete( name = waiver(), ..., - palette = seq_along, + palette = seq_len, expand = waiver(), guide = waiver(), position = "left" From f6fef113f83a843980da24867539d1441ffc2225 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 20 May 2024 13:05:45 +0200 Subject: [PATCH 8/9] rephrase palette arg --- R/scale-discrete-.R | 5 +++-- man/scale_discrete.Rd | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/R/scale-discrete-.R b/R/scale-discrete-.R index 684c549549..b957b97b97 100644 --- a/R/scale-discrete-.R +++ b/R/scale-discrete-.R @@ -12,8 +12,9 @@ #' #' @inheritDotParams discrete_scale -scale_name #' @inheritParams discrete_scale -#' @param palette A function that takes the limits as input and provides -#' numerical values as output. +#' @param palette A palette function that when called with a single integer +#' argument (the number of levels in the scale) returns the numerical values +#' that they should take. #' @param sec.axis [dup_axis()] is used to specify a secondary axis. #' @rdname scale_discrete #' @family position scales diff --git a/man/scale_discrete.Rd b/man/scale_discrete.Rd index 8cb7f0644a..381c09254e 100644 --- a/man/scale_discrete.Rd +++ b/man/scale_discrete.Rd @@ -78,8 +78,9 @@ notation. \item{\code{super}}{The super class to use for the constructed scale} }} -\item{palette}{A function that takes the limits as input and provides -numerical values as output.} +\item{palette}{A palette function that when called with a single integer +argument (the number of levels in the scale) returns the numerical values +that they should take.} \item{expand}{For position scales, a vector of range expansion constants used to add some padding around the data to ensure that they are placed some distance From db0dfdc940612c2ebd9670bdcf7d083ee9144b67 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 20 May 2024 13:07:35 +0200 Subject: [PATCH 9/9] add news bullet --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index 633a46172b..4e8216504e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # ggplot2 (development version) +* Discrete position scales now expose the `palette` argument, which can be used + to customise spacings between levels (@teunbrand, #5770). * The default `se` parameter in layers with `geom = "smooth"` will be `TRUE` when the data has `ymin` and `ymax` parameters and `FALSE` if these are absent. Note that this does not affect the default of `geom_smooth()` or