From 9262f3d584f00750bcc86b6af81a4e623925a15f Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Thu, 7 Dec 2023 10:37:20 +0100 Subject: [PATCH 1/8] reimplement #3591 --- R/scale-.R | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/R/scale-.R b/R/scale-.R index f559dbb37d..3e187d0b5d 100644 --- a/R/scale-.R +++ b/R/scale-.R @@ -744,8 +744,18 @@ ScaleContinuous <- ggproto("ScaleContinuous", Scale, breaks <- self$trans$minor_breaks(b, limits, n) } } else if (is.function(self$minor_breaks)) { - # Find breaks in data space, and convert to numeric - breaks <- self$minor_breaks(self$trans$inverse(limits)) + # Using `fetch_ggproto` here to avoid auto-wrapping the user-supplied + # breaks function as a ggproto method. + break_fun <- fetch_ggproto(self, "minor_breaks") + arg_names <- fn_fmls_names(break_fun) + + # Find breaks in data space + if (length(arg_names) == 1L) { + breaks <- break_fun(self$trans$inverse(limits)) + } else { + breaks <- break_fun(self$trans$inverse(limits), self$trans$inverse(b)) + } + # Convert breaks to numeric breaks <- self$trans$transform(breaks) } else { breaks <- self$trans$transform(self$minor_breaks) From 802d9ed77622f4bb9e2a306f4192224f1edd0acf Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Thu, 7 Dec 2023 10:38:22 +0100 Subject: [PATCH 2/8] Do not censor major breaks --- R/scale-.R | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/R/scale-.R b/R/scale-.R index 3e187d0b5d..2223065905 100644 --- a/R/scale-.R +++ b/R/scale-.R @@ -714,11 +714,7 @@ ScaleContinuous <- ggproto("ScaleContinuous", Scale, } # Breaks in data space need to be converted back to transformed space - breaks <- self$trans$transform(breaks) - # Any breaks outside the dimensions are flagged as missing - breaks <- censor(breaks, self$trans$transform(limits), only.finite = FALSE) - - breaks + self$trans$transform(breaks) }, get_breaks_minor = function(self, n = 2, b = self$break_positions(), limits = self$get_limits()) { @@ -736,6 +732,9 @@ ScaleContinuous <- ggproto("ScaleContinuous", Scale, call = self$call ) } + # major breaks are not censored, however; + # some transforms assume finite major breaks + b <- b[is.finite(b)] if (is.waive(self$minor_breaks)) { if (is.null(b)) { @@ -829,14 +828,16 @@ ScaleContinuous <- ggproto("ScaleContinuous", Scale, # labels labels <- self$get_labels(major) - # drop oob breaks/labels by testing major == NA - if (!is.null(labels)) labels <- labels[!is.na(major)] - if (!is.null(major)) major <- major[!is.na(major)] - # minor breaks minor <- self$get_breaks_minor(b = major, limits = range) if (!is.null(minor)) minor <- minor[!is.na(minor)] + major <- oob_censor_any(major, range) + + # drop oob breaks/labels by testing major == NA + if (!is.null(labels)) labels <- labels[!is.na(major)] + if (!is.null(major)) major <- major[!is.na(major)] + # rescale breaks [0, 1], which are used by coord/guide major_n <- rescale(major, from = range) minor_n <- rescale(minor, from = range) From 0b3869b42c86ec70d1fe5aa13c704f8c2074f3b7 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Thu, 7 Dec 2023 10:39:09 +0100 Subject: [PATCH 3/8] view scales censor major breaks after calculation of minor breaks --- R/scale-view.R | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/R/scale-view.R b/R/scale-view.R index 34af3181d8..1402a3ffee 100644 --- a/R/scale-view.R +++ b/R/scale-view.R @@ -21,10 +21,12 @@ view_scale_primary <- function(scale, limits = scale$get_limits(), continuous_scale_sorted <- sort(continuous_range) breaks <- scale$get_breaks(continuous_scale_sorted) minor_breaks <- scale$get_breaks_minor(b = breaks, limits = continuous_scale_sorted) + breaks <- censor(breaks, continuous_scale_sorted, only.finite = FALSE) } else { breaks <- scale$get_breaks(limits) minor_breaks <- scale$get_breaks_minor(b = breaks, limits = limits) } + minor_breaks <- censor(minor_breaks, continuous_range, only.finite = FALSE) ggproto(NULL, ViewScale, scale = scale, @@ -76,7 +78,7 @@ view_scale_secondary <- function(scale, limits = scale$get_limits(), aesthetics = scale$aesthetics, name = scale$sec_name(), make_title = function(self, title) self$scale$make_sec_title(title), - + continuous_range = sort(continuous_range), dimension = function(self) self$break_info$range, get_limits = function(self) self$break_info$range, get_breaks = function(self) self$break_info$major_source, From 931b3451975d337a91e9afd4f4c65bc83994cdc9 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Thu, 7 Dec 2023 10:41:08 +0100 Subject: [PATCH 4/8] guides censor breaks --- R/guide-.R | 3 ++- R/guide-bins.R | 1 + R/guide-colorsteps.R | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/R/guide-.R b/R/guide-.R index cdb750ce56..85fb5ee942 100644 --- a/R/guide-.R +++ b/R/guide-.R @@ -224,7 +224,8 @@ Guide <- ggproto( key$.label <- labels if (is.numeric(breaks)) { - vec_slice(key, is.finite(breaks)) + range <- scale$continuous_range %||% scale$get_limits() + key <- vec_slice(key, is.finite(oob_censor_any(breaks, range))) } else { key } diff --git a/R/guide-bins.R b/R/guide-bins.R index 77ea847b53..fd53ff11d5 100644 --- a/R/guide-bins.R +++ b/R/guide-bins.R @@ -262,6 +262,7 @@ GuideBins <- ggproto( } key$.label <- labels + key <- vec_slice(key, !is.na(oob_censor_any(key$.value))) return(key) }, diff --git a/R/guide-colorsteps.R b/R/guide-colorsteps.R index c5315e6da6..7206a4c19e 100644 --- a/R/guide-colorsteps.R +++ b/R/guide-colorsteps.R @@ -181,6 +181,8 @@ GuideColoursteps <- ggproto( params$key$.value <- rescale(params$key$.value, from = limits) params$decor$min <- rescale(params$decor$min, from = limits) params$decor$max <- rescale(params$decor$max, from = limits) + params$key <- + vec_slice(params$key, !is.na(oob_censor_any(params$key$.value))) params }, From a6773a36d4145002713314a4b8a1c00cadfc27e2 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Thu, 7 Dec 2023 10:41:41 +0100 Subject: [PATCH 5/8] tests expect non-censored breaks --- tests/testthat/test-scales-breaks-labels.R | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/testthat/test-scales-breaks-labels.R b/tests/testthat/test-scales-breaks-labels.R index 344d823d58..b3619ec2df 100644 --- a/tests/testthat/test-scales-breaks-labels.R +++ b/tests/testthat/test-scales-breaks-labels.R @@ -1,7 +1,7 @@ test_that("labels match breaks, even when outside limits", { sc <- scale_y_continuous(breaks = 1:4, labels = 1:4, limits = c(1, 3)) - expect_equal(sc$get_breaks(), c(1:3, NA)) + expect_equal(sc$get_breaks(), 1:4) expect_equal(sc$get_labels(), 1:4) expect_equal(sc$get_breaks_minor(), c(1, 1.5, 2, 2.5, 3)) }) @@ -231,7 +231,7 @@ test_that("breaks can be specified by names of labels", { test_that("only finite or NA values for breaks for transformed scales (#871)", { sc <- scale_y_continuous(limits = c(0.01, 0.99), trans = "probit", breaks = seq(0, 1, 0.2)) - breaks <- sc$get_breaks() + breaks <- sc$break_info()$major_soruce expect_true(all(is.finite(breaks) | is.na(breaks))) }) @@ -257,7 +257,7 @@ test_that("equal length breaks and labels can be passed to ViewScales with limit limits = c(10, 30) ) - expect_identical(test_scale$get_breaks(), c(NA, 20, NA)) + expect_identical(test_scale$get_breaks(), c(0, 20, 40)) expect_identical(test_scale$get_labels(), c(c("0", "20", "40"))) test_view_scale <- view_scale_primary(test_scale) From 445e4848050961fdc7dd053ffeab480841ce9480 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Thu, 7 Dec 2023 10:41:49 +0100 Subject: [PATCH 6/8] add news bullet --- NEWS.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/NEWS.md b/NEWS.md index cabdc6940b..1bc31a921b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,11 @@ # ggplot2 (development version) +* The `minor_breaks` function argument in scales can now take a function with + two arguments: the scale's limits and the scale's major breaks (#3583). + +* (internal) The `ScaleContinuous$get_breaks()` method no longer censors + the computed breaks. + * The plot's title, subtitle and caption now obey horizontal text margins (#5533). From 45003431176c6e06b57eb5fc8a50b6334e3bd941 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Thu, 7 Dec 2023 10:49:33 +0100 Subject: [PATCH 7/8] adjust docs --- R/scale-.R | 4 +++- man/continuous_scale.Rd | 4 +++- man/scale_continuous.Rd | 4 +++- man/scale_gradient.Rd | 4 +++- man/scale_size.Rd | 4 +++- 5 files changed, 15 insertions(+), 5 deletions(-) diff --git a/R/scale-.R b/R/scale-.R index 2223065905..d4a4eaa857 100644 --- a/R/scale-.R +++ b/R/scale-.R @@ -26,7 +26,9 @@ #' each major break) #' - A numeric vector of positions #' - A function that given the limits returns a vector of minor breaks. Also -#' accepts rlang [lambda][rlang::as_function()] function notation. +#' accepts rlang [lambda][rlang::as_function()] function notation. When +#' the function has two arguments, it will be given the limits and major +#' breaks. #' @param n.breaks An integer guiding the number of major breaks. The algorithm #' may choose a slightly different number to ensure nice break labels. Will #' only have an effect if `breaks = waiver()`. Use `NULL` to use the default diff --git a/man/continuous_scale.Rd b/man/continuous_scale.Rd index 407916b251..6bdc511f13 100644 --- a/man/continuous_scale.Rd +++ b/man/continuous_scale.Rd @@ -58,7 +58,9 @@ Also accepts rlang \link[rlang:as_function]{lambda} function notation. each major break) \item A numeric vector of positions \item A function that given the limits returns a vector of minor breaks. Also -accepts rlang \link[rlang:as_function]{lambda} function notation. +accepts rlang \link[rlang:as_function]{lambda} function notation. When +the function has two arguments, it will be given the limits and major +breaks. }} \item{n.breaks}{An integer guiding the number of major breaks. The algorithm diff --git a/man/scale_continuous.Rd b/man/scale_continuous.Rd index ec91afc919..da226139d7 100644 --- a/man/scale_continuous.Rd +++ b/man/scale_continuous.Rd @@ -79,7 +79,9 @@ Also accepts rlang \link[rlang:as_function]{lambda} function notation. each major break) \item A numeric vector of positions \item A function that given the limits returns a vector of minor breaks. Also -accepts rlang \link[rlang:as_function]{lambda} function notation. +accepts rlang \link[rlang:as_function]{lambda} function notation. When +the function has two arguments, it will be given the limits and major +breaks. }} \item{n.breaks}{An integer guiding the number of major breaks. The algorithm diff --git a/man/scale_gradient.Rd b/man/scale_gradient.Rd index cea9e781f9..379476681b 100644 --- a/man/scale_gradient.Rd +++ b/man/scale_gradient.Rd @@ -114,7 +114,9 @@ Also accepts rlang \link[rlang:as_function]{lambda} function notation. each major break) \item A numeric vector of positions \item A function that given the limits returns a vector of minor breaks. Also -accepts rlang \link[rlang:as_function]{lambda} function notation. +accepts rlang \link[rlang:as_function]{lambda} function notation. When +the function has two arguments, it will be given the limits and major +breaks. }} \item{\code{n.breaks}}{An integer guiding the number of major breaks. The algorithm may choose a slightly different number to ensure nice break labels. Will diff --git a/man/scale_size.Rd b/man/scale_size.Rd index 408493113f..d8b92414c7 100644 --- a/man/scale_size.Rd +++ b/man/scale_size.Rd @@ -131,7 +131,9 @@ breaks are given explicitly.} each major break) \item A numeric vector of positions \item A function that given the limits returns a vector of minor breaks. Also -accepts rlang \link[rlang:as_function]{lambda} function notation. +accepts rlang \link[rlang:as_function]{lambda} function notation. When +the function has two arguments, it will be given the limits and major +breaks. }} \item{\code{oob}}{One of: \itemize{ From 0f784e51a4032c386aa0c8658862b112dfb29be2 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Thu, 7 Dec 2023 10:53:26 +0100 Subject: [PATCH 8/8] fix typo --- tests/testthat/test-scales-breaks-labels.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/test-scales-breaks-labels.R b/tests/testthat/test-scales-breaks-labels.R index b3619ec2df..a83ed63498 100644 --- a/tests/testthat/test-scales-breaks-labels.R +++ b/tests/testthat/test-scales-breaks-labels.R @@ -231,7 +231,7 @@ test_that("breaks can be specified by names of labels", { test_that("only finite or NA values for breaks for transformed scales (#871)", { sc <- scale_y_continuous(limits = c(0.01, 0.99), trans = "probit", breaks = seq(0, 1, 0.2)) - breaks <- sc$break_info()$major_soruce + breaks <- sc$break_info()$major_source expect_true(all(is.finite(breaks) | is.na(breaks))) })