From 6fefc8979c4e2f07899c10521a525fc4a4627665 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Thu, 20 Jun 2019 16:48:44 -0400 Subject: [PATCH 01/26] move expantion-related functions to their own file --- DESCRIPTION | 3 +- R/coord-sf.R | 11 -- R/scale-expantion.r | 144 ++++++++++++++++++++++++++ R/utilities.r | 71 ------------- man/expand_range4.Rd | 22 ++++ man/expand_scale.Rd | 13 ++- tests/testthat/test-scale-expantion.r | 20 ++++ 7 files changed, 194 insertions(+), 90 deletions(-) create mode 100644 R/scale-expantion.r create mode 100644 man/expand_range4.Rd create mode 100644 tests/testthat/test-scale-expantion.r diff --git a/DESCRIPTION b/DESCRIPTION index 3b98796c3b..d035d5f6a8 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -54,7 +54,7 @@ Suggests: rpart, sf (>= 0.7-3), svglite (>= 1.2.0.9001), - testthat (>= 0.11.0), + testthat (>= 2.1.0), vdiffr (>= 0.3.0) Enhances: sp License: GPL-2 | file LICENSE @@ -190,6 +190,7 @@ Collate: 'scale-continuous.r' 'scale-date.r' 'scale-discrete-.r' + 'scale-expantion.r' 'scale-gradient.r' 'scale-grey.r' 'scale-hue.r' diff --git a/R/coord-sf.R b/R/coord-sf.R index 00d932b9fb..20b0d35b79 100644 --- a/R/coord-sf.R +++ b/R/coord-sf.R @@ -466,14 +466,3 @@ parse_axes_labeling <- function(x) { labs = unlist(strsplit(x, "")) list(top = labs[1], right = labs[2], bottom = labs[3], left = labs[4]) } - -scale_range <- function(scale, limits = NULL, expand = TRUE) { - expansion <- if (expand) expand_default(scale) else expand_scale(0, 0) - - if (is.null(limits)) { - scale$dimension(expansion) - } else { - continuous_range <- range(scale$transform(limits)) - expand_range4(continuous_range, expansion) - } -} diff --git a/R/scale-expantion.r b/R/scale-expantion.r new file mode 100644 index 0000000000..a418ba2c18 --- /dev/null +++ b/R/scale-expantion.r @@ -0,0 +1,144 @@ + +#' Generate expansion vector for scales +#' +#' This is a convenience function for generating scale expansion vectors +#' for the `expand` argument of [scale_(x|y)_continuous][scale_x_continuous()] +#' and [scale_(x|y)_discrete][scale_x_discrete()]. The expansions vectors are used to +#' add some space between the data and the axes. +#' +#' @param mult vector of multiplicative range expansion factors. +#' If length 1, both the lower and upper limits of the scale +#' are expanded outwards by `mult`. If length 2, the lower limit +#' is expanded by `mult[1]` and the upper limit by `mult[2]`. +#' @param add vector of additive range expansion constants. +#' If length 1, both the lower and upper limits of the scale +#' are expanded outwards by `add` units. If length 2, the +#' lower limit is expanded by `add[1]` and the upper +#' limit by `add[2]`. +#' +#' @export +#' @examples +#' # No space below the bars but 10% above them +#' ggplot(mtcars) + +#' geom_bar(aes(x = factor(cyl))) + +#' scale_y_continuous(expand = expand_scale(mult = c(0, .1))) +#' +#' # Add 2 units of space on the left and right of the data +#' ggplot(subset(diamonds, carat > 2), aes(cut, clarity)) + +#' geom_jitter() + +#' scale_x_discrete(expand = expand_scale(add = 2)) +#' +#' # Reproduce the default range expansion used +#' # when the 'expand' argument is not specified +#' ggplot(subset(diamonds, carat > 2), aes(cut, price)) + +#' geom_jitter() + +#' scale_x_discrete(expand = expand_scale(add = .6)) + +#' scale_y_continuous(expand = expand_scale(mult = .05)) +#' +expand_scale = function(mult = 0, add = 0) { + stopifnot(is.numeric(mult) && is.numeric(add)) + stopifnot((length(mult) %in% 1:2) && (length(add) %in% 1:2)) + + mult <- rep(mult, length.out = 2) + add <- rep(add, length.out = 2) + c(mult[1], add[1], mult[2], add[2]) +} + + +#' Calculate limits for continuous scales +#' +#' @param limits a numeric vector of length 2 or a function that will be +#' called on `default_limits` that returns a numeric vector of length 2. +#' The `limits` are in transformed data space. +#' @param default_limits a numeric vector of length 2 to be used as a +#' fallback for `NA` limits. This is usually the minimum and maximum +#' value observed in the transformed data. +#' @param trans The transform (see [scales::trans_new()]) that was used +#' to calculate `limits` and `default_limits`. +#' +#' @noRd +#' +calculate_limits <- function(limits = c(NA_real_, NA_real_), + default_limits = c(NA_real_, NA_real_), + trans = identity_trans()) { + + if (!is.numeric(default_limits) || (length(default_limits) != 2)) { + stop("`default_limits` must be a numeric vector of length 2") + } + + if (is.function(limits)) { + # if limits is a function, it expects to work in trans$inverse() space + limits <- trans$transform(limits(trans$inverse(default_limits))) + } + + if (!is.numeric(limits) || (length(limits) != 2)) { + stop( + "`limits` must be a numeric vector of length 2 or a function of `default_limits` ", + "that returns a numeric vector of length 2.", + call. = FALSE + ) + } + + # replace NA limits with corresponding value in default_limits + ifelse(is.na(limits), default_limits, limits) +} + +#' Expand a numeric range +#' +#' @param limits A numeric vector of length 2 giving the +#' range to expand. +#' @param expand A numeric vector of length 2 (`c(add, mult)`) +#' or length 4 (`c(mult_left, add_left, mult_right, add_right)`), +#' as generated by [expand_scale()]. +#' +#' @return The expanded `limits` +#' +expand_range4 <- function(limits, expand) { + stopifnot( + is.numeric(expand), + length(expand) %in% c(2,4) + ) + + # If only two expansion constants are given (i.e. the old syntax), + # reuse them to generate a four-element expansion vector + if (length(expand) == 2) { + expand <- c(expand, expand) + } + + # Calculate separate range expansion for the lower and + # upper range limits, and then combine them into one vector + lower <- expand_range(limits, expand[1], expand[2])[1] + upper <- expand_range(limits, expand[3], expand[4])[2] + c(lower, upper) +} + + +#' Calculate the final range +#' +#' @param scale A position scale (e.g., [scale_x_continuous()] or [scale_x_discrete()]) +#' @param limits The user-supplied limits, in untransformed data space +#' @param expand `TRUE` of the final limits should be expanded, `FALSE` otherwise +#' +#' @return The (possibly expanded) `limits` +#' @noRd +#' +scale_range <- function(scale, limits = NULL, expand = TRUE) { + expansion <- if (expand) expand_default(scale) else expand_scale(0, 0) + + if (is.null(limits)) { + scale$dimension(expansion) + } else { + continuous_range <- range(scale$transform(limits)) + expand_range4(continuous_range, expansion) + } +} + + + +expand_default <- function(scale, discrete = c(0, 0.6, 0, 0.6), continuous = c(0.05, 0, 0.05, 0)) { + if (!is.waive(scale$expand)) { + return(scale$expand) + } + + if (scale$is_discrete()) discrete else continuous +} diff --git a/R/utilities.r b/R/utilities.r index db8c16f932..9c0ac8bd5d 100644 --- a/R/utilities.r +++ b/R/utilities.r @@ -176,77 +176,6 @@ rescale01 <- function(x) { (x - rng[1]) / (rng[2] - rng[1]) } -#' Similar to expand_range(), but taking a vector ‘expand’ -#' of *four* expansion values, where the 1st and 2nd -#' elements are used for the lower limit, and the 3rd and -#' 4th elements are used for the upper limit). -#' -#' The ‘expand’ argument can also be of length 2, -#' and the expansion values for the lower limit -#' are then reused for the upper limit. -# -#' @noRd -#' @keywords internal -expand_range4 <- function(limits, expand) { - stopifnot(is.numeric(expand) && (length(expand) %in% c(2,4))) - # If only two expansion constants are given (i.e. the old syntax), - # reuse them to generate a four-element expansion vector - if (length(expand) == 2) { expand <- c(expand, expand) } - - # Calculate separate range expansion for the lower and - # upper range limits, and then combine them into one vector - lower <- expand_range(limits, expand[1], expand[2])[1] - upper <- expand_range(limits, expand[3], expand[4])[2] - c(lower, upper) -} - -#' Generate expansion vector for scales. -#' -#' This is a convenience function for generating scale expansion vectors -#' for the \code{expand} argument of -#' \code{\link[=scale_x_continuous]{scale_*_continuous}} and -#' \code{\link[=scale_x_discrete]{scale_*_discrete}}. -#' The expansions vectors are used to add some space between -#' the data and the axes. -#' -#' @export -#' @param mult vector of multiplicative range expansion factors. -#' If length 1, both the lower and upper limits of the scale -#' are expanded outwards by \code{mult}. If length 2, the lower limit -#' is expanded by \code{mult[1]} and the upper limit by \code{mult[2]}. -#' @param add vector of additive range expansion constants. -#' If length 1, both the lower and upper limits of the scale -#' are expanded outwards by \code{add} units. If length 2, the -#' lower limit is expanded by \code{add[1]} and the upper -#' limit by \code{add[2]}. -#' @examples -#' # No space below the bars but 10% above them -#' ggplot(mtcars) + -#' geom_bar(aes(x = factor(cyl))) + -#' scale_y_continuous(expand = expand_scale(mult = c(0, .1))) -#' -#' # Add 2 units of space on the left and right of the data -#' ggplot(subset(diamonds, carat > 2), aes(cut, clarity)) + -#' geom_jitter() + -#' scale_x_discrete(expand = expand_scale(add = 2)) -#' -#' # Reproduce the default range expansion used -#' # when the 'expand' argument is not specified -#' ggplot(subset(diamonds, carat > 2), aes(cut, price)) + -#' geom_jitter() + -#' scale_x_discrete(expand = expand_scale(add = .6)) + -#' scale_y_continuous(expand = expand_scale(mult = .05)) -expand_scale = function(mult = 0, add = 0) { - stopifnot(is.numeric(mult) && is.numeric(add)) - stopifnot((length(mult) %in% 1:2) && (length(add) %in% 1:2)) - - mult <- rep(mult, length.out = 2) - add <- rep(add, length.out = 2) - c(mult[1], add[1], mult[2], add[2]) -} - - - #' Give a deprecation error, warning, or message, depending on version number. #' #' Version numbers have the format .., like 0.9.2. diff --git a/man/expand_range4.Rd b/man/expand_range4.Rd new file mode 100644 index 0000000000..44bd871910 --- /dev/null +++ b/man/expand_range4.Rd @@ -0,0 +1,22 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/scale-expantion.r +\name{expand_range4} +\alias{expand_range4} +\title{Expand a numeric range} +\usage{ +expand_range4(limits, expand) +} +\arguments{ +\item{limits}{A numeric vector of length 2 giving the +range to expand.} + +\item{expand}{A numeric vector of length 2 (\code{c(add, mult)}) +or length 4 (\code{c(mult_left, add_left, mult_right, add_right)}), +as generated by \code{\link[=expand_scale]{expand_scale()}}.} +} +\value{ +The expanded \code{limits} +} +\description{ +Expand a numeric range +} diff --git a/man/expand_scale.Rd b/man/expand_scale.Rd index f7ddb6b770..c148322b3b 100644 --- a/man/expand_scale.Rd +++ b/man/expand_scale.Rd @@ -1,8 +1,8 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/utilities.r +% Please edit documentation in R/scale-expantion.r \name{expand_scale} \alias{expand_scale} -\title{Generate expansion vector for scales.} +\title{Generate expansion vector for scales} \usage{ expand_scale(mult = 0, add = 0) } @@ -20,11 +20,9 @@ limit by \code{add[2]}.} } \description{ This is a convenience function for generating scale expansion vectors -for the \code{expand} argument of -\code{\link[=scale_x_continuous]{scale_*_continuous}} and -\code{\link[=scale_x_discrete]{scale_*_discrete}}. -The expansions vectors are used to add some space between -the data and the axes. +for the \code{expand} argument of \link[=scale_x_continuous]{scale_(x|y)_continuous} +and \link[=scale_x_discrete]{scale_(x|y)_discrete}. The expansions vectors are used to +add some space between the data and the axes. } \examples{ # No space below the bars but 10\% above them @@ -43,4 +41,5 @@ ggplot(subset(diamonds, carat > 2), aes(cut, price)) + geom_jitter() + scale_x_discrete(expand = expand_scale(add = .6)) + scale_y_continuous(expand = expand_scale(mult = .05)) + } diff --git a/tests/testthat/test-scale-expantion.r b/tests/testthat/test-scale-expantion.r new file mode 100644 index 0000000000..f9e0116871 --- /dev/null +++ b/tests/testthat/test-scale-expantion.r @@ -0,0 +1,20 @@ + +test_that("calculate_limits() with all NA limits returns the default limits", { + expect_identical(calculate_limits(c(NA_real_, NA_real_), default_limits = c(0, 1)), c(0, 1)) +}) + +test_that("functional limits operate on the inverse-transformed default limits in calculate_limits()", { + expect_identical( + calculate_limits( + limits = function(x) x * 5, + default_limits = log10(c(1, 2)), + trans = log10_trans() + ), + log10(c(5, 10)) + ) +}) + +test_that("invalid inputs to calculate_limits() are detected", { + expect_error(calculate_limits(default_limits = NULL), "numeric vector of length 2") + expect_error(calculate_limits(limits = NULL), "numeric vector of length 2") +}) From 913e440730d44cb8b09e61d1edeecb2eb66e13e0 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Fri, 21 Jun 2019 13:32:32 -0400 Subject: [PATCH 02/26] fix spelling of expansion in filenames, move expand_default to scale-expansion.r add and test the new scale_dimension() function --- DESCRIPTION | 2 +- R/coord-.r | 4 - R/{scale-expantion.r => scale-expansion.r} | 92 +++++++++++----------- man/expand_range4.Rd | 2 +- man/expand_scale.Rd | 2 +- tests/testthat/test-scale-expansion.r | 50 ++++++++++++ tests/testthat/test-scale-expantion.r | 20 ----- 7 files changed, 100 insertions(+), 72 deletions(-) rename R/{scale-expantion.r => scale-expansion.r} (60%) create mode 100644 tests/testthat/test-scale-expansion.r delete mode 100644 tests/testthat/test-scale-expantion.r diff --git a/DESCRIPTION b/DESCRIPTION index d035d5f6a8..7969bc5f43 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -190,7 +190,7 @@ Collate: 'scale-continuous.r' 'scale-date.r' 'scale-discrete-.r' - 'scale-expantion.r' + 'scale-expansion.r' 'scale-gradient.r' 'scale-grey.r' 'scale-hue.r' diff --git a/R/coord-.r b/R/coord-.r index 68c48d5291..e2ea1a025d 100644 --- a/R/coord-.r +++ b/R/coord-.r @@ -126,10 +126,6 @@ Coord <- ggproto("Coord", #' @keywords internal is.Coord <- function(x) inherits(x, "Coord") -expand_default <- function(scale, discrete = c(0, 0.6, 0, 0.6), continuous = c(0.05, 0, 0.05, 0)) { - scale$expand %|W|% if (scale$is_discrete()) discrete else continuous -} - # Renders an axis with the correct orientation or zeroGrob if no axis should be # generated render_axis <- function(panel_params, axis, scale, position, theme) { diff --git a/R/scale-expantion.r b/R/scale-expansion.r similarity index 60% rename from R/scale-expantion.r rename to R/scale-expansion.r index a418ba2c18..553bfcd332 100644 --- a/R/scale-expantion.r +++ b/R/scale-expansion.r @@ -44,45 +44,6 @@ expand_scale = function(mult = 0, add = 0) { c(mult[1], add[1], mult[2], add[2]) } - -#' Calculate limits for continuous scales -#' -#' @param limits a numeric vector of length 2 or a function that will be -#' called on `default_limits` that returns a numeric vector of length 2. -#' The `limits` are in transformed data space. -#' @param default_limits a numeric vector of length 2 to be used as a -#' fallback for `NA` limits. This is usually the minimum and maximum -#' value observed in the transformed data. -#' @param trans The transform (see [scales::trans_new()]) that was used -#' to calculate `limits` and `default_limits`. -#' -#' @noRd -#' -calculate_limits <- function(limits = c(NA_real_, NA_real_), - default_limits = c(NA_real_, NA_real_), - trans = identity_trans()) { - - if (!is.numeric(default_limits) || (length(default_limits) != 2)) { - stop("`default_limits` must be a numeric vector of length 2") - } - - if (is.function(limits)) { - # if limits is a function, it expects to work in trans$inverse() space - limits <- trans$transform(limits(trans$inverse(default_limits))) - } - - if (!is.numeric(limits) || (length(limits) != 2)) { - stop( - "`limits` must be a numeric vector of length 2 or a function of `default_limits` ", - "that returns a numeric vector of length 2.", - call. = FALSE - ) - } - - # replace NA limits with corresponding value in default_limits - ifelse(is.na(limits), default_limits, limits) -} - #' Expand a numeric range #' #' @param limits A numeric vector of length 2 giving the @@ -112,12 +73,11 @@ expand_range4 <- function(limits, expand) { c(lower, upper) } - #' Calculate the final range #' #' @param scale A position scale (e.g., [scale_x_continuous()] or [scale_x_discrete()]) #' @param limits The user-supplied limits, in untransformed data space -#' @param expand `TRUE` of the final limits should be expanded, `FALSE` otherwise +#' @param expand `TRUE` if the final limits should be expanded, `FALSE` otherwise #' #' @return The (possibly expanded) `limits` #' @noRd @@ -133,12 +93,54 @@ scale_range <- function(scale, limits = NULL, expand = TRUE) { } } +#' Calculate the final scale dimension +#' +#' @param scale A position scale (e.g., [scale_x_continuous()] or [scale_x_discrete()]) +#' @param scale_limits The scale-supplied limits, in transformed data space +#' @param coord_limits The user-supplied limits, in untransformed data space +#' @param expansion The length-4 expansion to use, calculated by [expand_scale()] +#' or [expand_default()]. +#' @param coord_trans A transformation defining the space where +#' expansion should take place. +#' +#' @return The (possibly expanded) scale limits, in scale-transformed data space +#' @noRd +#' +scale_dimension <- function(scale, coord_limits = c(NA_real_, NA_real_), + expansion = expand_scale(0, 0), + coord_trans = identity_trans()) { + + # transform coord limits to transformed scale data space + coord_limits_scale <- scale$trans$transform(coord_limits) + + # replace scale limits with non-NA coord limits + scale_limits <- ifelse(is.na(coord_limits_scale), scale$dimension(), coord_limits_scale) + # expand limits in coordinate space + coord_limits <- coord_trans$transform(scale_limits) + coord_limits <- expand_range4(coord_limits, expansion) + final_scale_limits <- coord_trans$inverse(coord_limits) -expand_default <- function(scale, discrete = c(0, 0.6, 0, 0.6), continuous = c(0.05, 0, 0.05, 0)) { - if (!is.waive(scale$expand)) { - return(scale$expand) + # if any non-finite values were introduced in the transformations, + # replace them with the original scale limits + ifelse(is.finite(final_scale_limits), final_scale_limits, scale_limits) +} + +#' Calculate the default expansion for a scale +#' +#' @param scale A position scale (e.g., [scale_x_continuous()] or [scale_x_discrete()]) +#' @param discrete,continuous Default scale expansion factors for +#' discrete and continuous scales, respectively. +#' @param expand Should any expansion be applied? +#' +#' @return One of `discrete`, `continuous`, or `scale$expand` +#' @noRd +#' +expand_default <- function(scale, discrete = expand_scale(add = 0.6), + continuous = expand_scale(mult = 0.05), expand = TRUE) { + if (!expand) { + return(expand_scale(0, 0)) } - if (scale$is_discrete()) discrete else continuous + scale$expand %|W|% if (scale$is_discrete()) discrete else continuous } diff --git a/man/expand_range4.Rd b/man/expand_range4.Rd index 44bd871910..cfdb8a7d8d 100644 --- a/man/expand_range4.Rd +++ b/man/expand_range4.Rd @@ -1,5 +1,5 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/scale-expantion.r +% Please edit documentation in R/scale-expansion.r \name{expand_range4} \alias{expand_range4} \title{Expand a numeric range} diff --git a/man/expand_scale.Rd b/man/expand_scale.Rd index c148322b3b..31bec3d23e 100644 --- a/man/expand_scale.Rd +++ b/man/expand_scale.Rd @@ -1,5 +1,5 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/scale-expantion.r +% Please edit documentation in R/scale-expansion.r \name{expand_scale} \alias{expand_scale} \title{Generate expansion vector for scales} diff --git a/tests/testthat/test-scale-expansion.r b/tests/testthat/test-scale-expansion.r new file mode 100644 index 0000000000..adcab729fb --- /dev/null +++ b/tests/testthat/test-scale-expansion.r @@ -0,0 +1,50 @@ + +test_that("scale_continuous_limits() can override scale limits", { + scale <- scale_x_continuous(limits = c(1, 2)) + expect_identical(scale_continuous_limits(scale, c(NA, NA)), c(1, 2)) + expect_identical(scale_continuous_limits(scale, c(NA, 3)), c(1, 3)) + expect_identical(scale_continuous_limits(scale, c(3, NA)), c(3, 2)) +}) + +test_that("scale_continuous_limits() returns limits in transformed scale space", { + scale_log <- scale_x_log10(limits = c(1, 10)) + expect_identical(scale_continuous_limits(scale_log), log10(c(1, 10))) + + # user-supplied limits are in data space + expect_identical(scale_continuous_limits(scale_log, coord_limits = c(1, 100)), log10(c(1, 100))) +}) + +test_that("scale_continuous_limits() expands coord-supplied limits", { + scale <- scale_x_continuous(limits = c(1, 2)) + expect_identical( + scale_continuous_limits(scale, coord_limits = c(0, 4), expansion = expand_scale(add = 1)), + c(-1, 5) + ) +}) + +test_that("scale_continuous_limits() expands limits", { + scale <- scale_x_continuous(limits = c(1, 2)) + expect_identical(scale_continuous_limits(scale, expansion = expand_scale(add = 1)), c(0, 3)) + + # expansion takes place in coordinate space + expect_identical( + scale_continuous_limits(scale, expansion = expand_scale(add = 0.5), coord_trans = log10_trans()), + 10^(expand_range4(log10(c(1, 2)), expand_scale(add = 0.5))) + ) +}) + +test_that("scale_continuous_limits() coord_trans only affects expansion", { + scale_log <- scale_x_log10(limits = c(1, 10)) + expect_identical( + scale_continuous_limits(scale_log, coord_trans = reverse_trans()), + scale_continuous_limits(scale_log) + ) +}) + +test_that("introduced non-finite values fall back on scale limits", { + scale <- scale_x_continuous(limits = c(1, 100)) + expect_identical( + scale_continuous_limits(scale, expansion = expand_scale(add = 2), coord_trans = sqrt_trans()), + c(1, (sqrt(100) + 2)^2) + ) +}) diff --git a/tests/testthat/test-scale-expantion.r b/tests/testthat/test-scale-expantion.r deleted file mode 100644 index f9e0116871..0000000000 --- a/tests/testthat/test-scale-expantion.r +++ /dev/null @@ -1,20 +0,0 @@ - -test_that("calculate_limits() with all NA limits returns the default limits", { - expect_identical(calculate_limits(c(NA_real_, NA_real_), default_limits = c(0, 1)), c(0, 1)) -}) - -test_that("functional limits operate on the inverse-transformed default limits in calculate_limits()", { - expect_identical( - calculate_limits( - limits = function(x) x * 5, - default_limits = log10(c(1, 2)), - trans = log10_trans() - ), - log10(c(5, 10)) - ) -}) - -test_that("invalid inputs to calculate_limits() are detected", { - expect_error(calculate_limits(default_limits = NULL), "numeric vector of length 2") - expect_error(calculate_limits(limits = NULL), "numeric vector of length 2") -}) From 12e3e8a1410258639d85f678b2f37c55badc6f34 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Fri, 21 Jun 2019 13:47:23 -0400 Subject: [PATCH 03/26] remove scale_range(), fix scale_dimension() tests --- R/coord-sf.R | 4 ++-- R/scale-expansion.r | 26 +++++----------------- tests/testthat/test-scale-expansion.r | 32 +++++++++++++-------------- 3 files changed, 23 insertions(+), 39 deletions(-) diff --git a/R/coord-sf.R b/R/coord-sf.R index 20b0d35b79..139d06ac41 100644 --- a/R/coord-sf.R +++ b/R/coord-sf.R @@ -127,8 +127,8 @@ CoordSf <- ggproto("CoordSf", CoordCartesian, setup_panel_params = function(self, scale_x, scale_y, params = list()) { # Bounding box of the data - x_range <- scale_range(scale_x, self$limits$x, self$expand) - y_range <- scale_range(scale_y, self$limits$y, self$expand) + x_range <- scale_dimension(scale_x, self$limits$x, expand_default(scale_x, expand = self$expand)) + y_range <- scale_dimension(scale_y, self$limits$y, expand_default(scale_y, expand = self$expand)) bbox <- c( x_range[1], y_range[1], x_range[2], y_range[2] diff --git a/R/scale-expansion.r b/R/scale-expansion.r index 553bfcd332..2c00e6abe3 100644 --- a/R/scale-expansion.r +++ b/R/scale-expansion.r @@ -73,31 +73,13 @@ expand_range4 <- function(limits, expand) { c(lower, upper) } -#' Calculate the final range -#' -#' @param scale A position scale (e.g., [scale_x_continuous()] or [scale_x_discrete()]) -#' @param limits The user-supplied limits, in untransformed data space -#' @param expand `TRUE` if the final limits should be expanded, `FALSE` otherwise -#' -#' @return The (possibly expanded) `limits` -#' @noRd -#' -scale_range <- function(scale, limits = NULL, expand = TRUE) { - expansion <- if (expand) expand_default(scale) else expand_scale(0, 0) - - if (is.null(limits)) { - scale$dimension(expansion) - } else { - continuous_range <- range(scale$transform(limits)) - expand_range4(continuous_range, expansion) - } -} - #' Calculate the final scale dimension #' #' @param scale A position scale (e.g., [scale_x_continuous()] or [scale_x_discrete()]) #' @param scale_limits The scale-supplied limits, in transformed data space #' @param coord_limits The user-supplied limits, in untransformed data space +#' (or `NULL` for no user-supplied limits). `NA` values will fall back to the +#' scale limits. #' @param expansion The length-4 expansion to use, calculated by [expand_scale()] #' or [expand_default()]. #' @param coord_trans A transformation defining the space where @@ -106,10 +88,12 @@ scale_range <- function(scale, limits = NULL, expand = TRUE) { #' @return The (possibly expanded) scale limits, in scale-transformed data space #' @noRd #' -scale_dimension <- function(scale, coord_limits = c(NA_real_, NA_real_), +scale_dimension <- function(scale, coord_limits = NULL, expansion = expand_scale(0, 0), coord_trans = identity_trans()) { + coord_limits <- coord_limits %||% c(NA_real_, NA_real_) + # transform coord limits to transformed scale data space coord_limits_scale <- scale$trans$transform(coord_limits) diff --git a/tests/testthat/test-scale-expansion.r b/tests/testthat/test-scale-expansion.r index adcab729fb..2bbc41d527 100644 --- a/tests/testthat/test-scale-expansion.r +++ b/tests/testthat/test-scale-expansion.r @@ -1,50 +1,50 @@ -test_that("scale_continuous_limits() can override scale limits", { +test_that("scale_dimension() can override scale limits", { scale <- scale_x_continuous(limits = c(1, 2)) - expect_identical(scale_continuous_limits(scale, c(NA, NA)), c(1, 2)) - expect_identical(scale_continuous_limits(scale, c(NA, 3)), c(1, 3)) - expect_identical(scale_continuous_limits(scale, c(3, NA)), c(3, 2)) + expect_identical(scale_dimension(scale, c(NA, NA)), c(1, 2)) + expect_identical(scale_dimension(scale, c(NA, 3)), c(1, 3)) + expect_identical(scale_dimension(scale, c(3, NA)), c(3, 2)) }) -test_that("scale_continuous_limits() returns limits in transformed scale space", { +test_that("scale_dimension() returns limits in transformed scale space", { scale_log <- scale_x_log10(limits = c(1, 10)) - expect_identical(scale_continuous_limits(scale_log), log10(c(1, 10))) + expect_identical(scale_dimension(scale_log), log10(c(1, 10))) # user-supplied limits are in data space - expect_identical(scale_continuous_limits(scale_log, coord_limits = c(1, 100)), log10(c(1, 100))) + expect_identical(scale_dimension(scale_log, coord_limits = c(1, 100)), log10(c(1, 100))) }) -test_that("scale_continuous_limits() expands coord-supplied limits", { +test_that("scale_dimension() expands coord-supplied limits", { scale <- scale_x_continuous(limits = c(1, 2)) expect_identical( - scale_continuous_limits(scale, coord_limits = c(0, 4), expansion = expand_scale(add = 1)), + scale_dimension(scale, coord_limits = c(0, 4), expansion = expand_scale(add = 1)), c(-1, 5) ) }) -test_that("scale_continuous_limits() expands limits", { +test_that("scale_dimension() expands limits", { scale <- scale_x_continuous(limits = c(1, 2)) - expect_identical(scale_continuous_limits(scale, expansion = expand_scale(add = 1)), c(0, 3)) + expect_identical(scale_dimension(scale, expansion = expand_scale(add = 1)), c(0, 3)) # expansion takes place in coordinate space expect_identical( - scale_continuous_limits(scale, expansion = expand_scale(add = 0.5), coord_trans = log10_trans()), + scale_dimension(scale, expansion = expand_scale(add = 0.5), coord_trans = log10_trans()), 10^(expand_range4(log10(c(1, 2)), expand_scale(add = 0.5))) ) }) -test_that("scale_continuous_limits() coord_trans only affects expansion", { +test_that("scale_dimension() coord_trans only affects expansion", { scale_log <- scale_x_log10(limits = c(1, 10)) expect_identical( - scale_continuous_limits(scale_log, coord_trans = reverse_trans()), - scale_continuous_limits(scale_log) + scale_dimension(scale_log, coord_trans = reverse_trans()), + scale_dimension(scale_log) ) }) test_that("introduced non-finite values fall back on scale limits", { scale <- scale_x_continuous(limits = c(1, 100)) expect_identical( - scale_continuous_limits(scale, expansion = expand_scale(add = 2), coord_trans = sqrt_trans()), + scale_dimension(scale, expansion = expand_scale(add = 2), coord_trans = sqrt_trans()), c(1, (sqrt(100) + 2)^2) ) }) From 586d271ceb7c0cd09a7c11c14353b205ad20dc8c Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Fri, 21 Jun 2019 16:07:54 -0400 Subject: [PATCH 04/26] move all scale expansion to Scale$dimension() --- R/coord-cartesian-.r | 11 ++---- R/coord-sf.R | 4 +-- R/scale-.r | 11 ++++-- R/scale-discrete-.r | 14 +++++--- R/scale-expansion.r | 41 +++------------------- tests/testthat/test-scale-expansion.r | 50 --------------------------- 6 files changed, 26 insertions(+), 105 deletions(-) delete mode 100644 tests/testthat/test-scale-expansion.r diff --git a/R/coord-cartesian-.r b/R/coord-cartesian-.r index 8efedd271c..e356ace11b 100644 --- a/R/coord-cartesian-.r +++ b/R/coord-cartesian-.r @@ -137,16 +137,9 @@ CoordCartesian <- ggproto("CoordCartesian", Coord, ) view_scales_from_scale <- function(scale, coord_limits = NULL, expand = TRUE) { - expansion <- if (expand) expand_default(scale) else expand_scale(0, 0) + expansion <- expand_default(scale, expand = expand) limits <- scale$get_limits() - - if (is.null(coord_limits)) { - continuous_range <- scale$dimension(expansion, limits) - } else { - continuous_range <- range(scale$transform(coord_limits)) - continuous_range <- expand_range4(continuous_range, expansion) - } - + continuous_range <- scale$dimension(expansion, coord_limits = coord_limits) aesthetic <- scale$aesthetics[1] view_scales <- list( diff --git a/R/coord-sf.R b/R/coord-sf.R index 139d06ac41..7ea1905563 100644 --- a/R/coord-sf.R +++ b/R/coord-sf.R @@ -127,8 +127,8 @@ CoordSf <- ggproto("CoordSf", CoordCartesian, setup_panel_params = function(self, scale_x, scale_y, params = list()) { # Bounding box of the data - x_range <- scale_dimension(scale_x, self$limits$x, expand_default(scale_x, expand = self$expand)) - y_range <- scale_dimension(scale_y, self$limits$y, expand_default(scale_y, expand = self$expand)) + x_range <- scale_x$dimension(expand_default(scale_x, expand = self$expand), coord_limits = self$limits$x) + y_range <- scale_y$dimension(expand_default(scale_y, expand = self$expand), coord_limits = self$limits$y) bbox <- c( x_range[1], y_range[1], x_range[2], y_range[2] diff --git a/R/scale-.r b/R/scale-.r index 0d39d0f2e5..6c374101c5 100644 --- a/R/scale-.r +++ b/R/scale-.r @@ -380,7 +380,7 @@ Scale <- ggproto("Scale", NULL, } }, - dimension = function(self, expand = c(0, 0, 0, 0), limits = self$get_limits()) { + dimension = function(self, expand = expand_scale(0, 0), limits = self$get_limits()) { stop("Not implemented", call. = FALSE) }, @@ -487,7 +487,12 @@ ScaleContinuous <- ggproto("ScaleContinuous", Scale, self$rescaler(x, from = range) }, - dimension = function(self, expand = c(0, 0, 0, 0), limits = self$get_limits()) { + dimension = function(self, expand = expand_scale(0, 0), limits = self$get_limits(), + coord_limits = NULL) { + coord_limits <- coord_limits %||% self$trans$inverse(c(NA, NA)) + coord_limits_scale <- self$trans$transform(coord_limits) + limits <- ifelse(is.na(coord_limits_scale), limits, coord_limits_scale) + expand_range4(limits, expand) }, @@ -688,7 +693,7 @@ ScaleDiscrete <- ggproto("ScaleDiscrete", Scale, rescale(x, match(as.character(x), limits), from = range) }, - dimension = function(self, expand = c(0, 0, 0, 0), limits = self$get_limits()) { + dimension = function(self, expand = expand_scale(0, 0), limits = self$get_limits()) { expand_range4(c(1, length(limits)), expand) }, diff --git a/R/scale-discrete-.r b/R/scale-discrete-.r index 13c04be751..ae04d88e67 100644 --- a/R/scale-discrete-.r +++ b/R/scale-discrete-.r @@ -105,20 +105,26 @@ ScaleDiscretePosition <- ggproto("ScaleDiscretePosition", ScaleDiscrete, rescale(self$map(x, limits = limits), from = range) }, - dimension = function(self, expand = c(0, 0, 0, 0), limits = self$get_limits()) { + dimension = function(self, expand = expand_scale(0, 0), limits = self$get_limits(), + coord_limits = NULL) { + coord_limits <- coord_limits %||% c(NA_real_, NA_real_) + c_range <- self$range_c$range d_range <- limits if (self$is_empty()) { c(0, 1) } else if (is.null(self$range$range)) { # only continuous + c_range <- ifelse(is.na(coord_limits), c_range, coord_limits) + expand_range4(c_range, expand) + } else if (is.null(c_range) || any(!is.na(coord_limits))) { # only discrete or coord-limited + c_range <- ifelse(is.na(coord_limits), c(1, length(d_range)), coord_limits) expand_range4(c_range, expand) - } else if (is.null(c_range)) { # only discrete - expand_range4(c(1, length(d_range)), expand) } else { # both + discrete_c_range <- ifelse(is.na(coord_limits), c(1, length(d_range)), coord_limits) range( c_range, - expand_range4(c(1, length(d_range)), expand) + expand_range4(discrete_c_range, expand) ) } }, diff --git a/R/scale-expansion.r b/R/scale-expansion.r index 2c00e6abe3..e3b3f82407 100644 --- a/R/scale-expansion.r +++ b/R/scale-expansion.r @@ -60,6 +60,10 @@ expand_range4 <- function(limits, expand) { length(expand) %in% c(2,4) ) + if (all(!is.finite(limits))) { + return(c(-Inf, Inf)) + } + # If only two expansion constants are given (i.e. the old syntax), # reuse them to generate a four-element expansion vector if (length(expand) == 2) { @@ -73,43 +77,6 @@ expand_range4 <- function(limits, expand) { c(lower, upper) } -#' Calculate the final scale dimension -#' -#' @param scale A position scale (e.g., [scale_x_continuous()] or [scale_x_discrete()]) -#' @param scale_limits The scale-supplied limits, in transformed data space -#' @param coord_limits The user-supplied limits, in untransformed data space -#' (or `NULL` for no user-supplied limits). `NA` values will fall back to the -#' scale limits. -#' @param expansion The length-4 expansion to use, calculated by [expand_scale()] -#' or [expand_default()]. -#' @param coord_trans A transformation defining the space where -#' expansion should take place. -#' -#' @return The (possibly expanded) scale limits, in scale-transformed data space -#' @noRd -#' -scale_dimension <- function(scale, coord_limits = NULL, - expansion = expand_scale(0, 0), - coord_trans = identity_trans()) { - - coord_limits <- coord_limits %||% c(NA_real_, NA_real_) - - # transform coord limits to transformed scale data space - coord_limits_scale <- scale$trans$transform(coord_limits) - - # replace scale limits with non-NA coord limits - scale_limits <- ifelse(is.na(coord_limits_scale), scale$dimension(), coord_limits_scale) - - # expand limits in coordinate space - coord_limits <- coord_trans$transform(scale_limits) - coord_limits <- expand_range4(coord_limits, expansion) - final_scale_limits <- coord_trans$inverse(coord_limits) - - # if any non-finite values were introduced in the transformations, - # replace them with the original scale limits - ifelse(is.finite(final_scale_limits), final_scale_limits, scale_limits) -} - #' Calculate the default expansion for a scale #' #' @param scale A position scale (e.g., [scale_x_continuous()] or [scale_x_discrete()]) diff --git a/tests/testthat/test-scale-expansion.r b/tests/testthat/test-scale-expansion.r deleted file mode 100644 index 2bbc41d527..0000000000 --- a/tests/testthat/test-scale-expansion.r +++ /dev/null @@ -1,50 +0,0 @@ - -test_that("scale_dimension() can override scale limits", { - scale <- scale_x_continuous(limits = c(1, 2)) - expect_identical(scale_dimension(scale, c(NA, NA)), c(1, 2)) - expect_identical(scale_dimension(scale, c(NA, 3)), c(1, 3)) - expect_identical(scale_dimension(scale, c(3, NA)), c(3, 2)) -}) - -test_that("scale_dimension() returns limits in transformed scale space", { - scale_log <- scale_x_log10(limits = c(1, 10)) - expect_identical(scale_dimension(scale_log), log10(c(1, 10))) - - # user-supplied limits are in data space - expect_identical(scale_dimension(scale_log, coord_limits = c(1, 100)), log10(c(1, 100))) -}) - -test_that("scale_dimension() expands coord-supplied limits", { - scale <- scale_x_continuous(limits = c(1, 2)) - expect_identical( - scale_dimension(scale, coord_limits = c(0, 4), expansion = expand_scale(add = 1)), - c(-1, 5) - ) -}) - -test_that("scale_dimension() expands limits", { - scale <- scale_x_continuous(limits = c(1, 2)) - expect_identical(scale_dimension(scale, expansion = expand_scale(add = 1)), c(0, 3)) - - # expansion takes place in coordinate space - expect_identical( - scale_dimension(scale, expansion = expand_scale(add = 0.5), coord_trans = log10_trans()), - 10^(expand_range4(log10(c(1, 2)), expand_scale(add = 0.5))) - ) -}) - -test_that("scale_dimension() coord_trans only affects expansion", { - scale_log <- scale_x_log10(limits = c(1, 10)) - expect_identical( - scale_dimension(scale_log, coord_trans = reverse_trans()), - scale_dimension(scale_log) - ) -}) - -test_that("introduced non-finite values fall back on scale limits", { - scale <- scale_x_continuous(limits = c(1, 100)) - expect_identical( - scale_dimension(scale, expansion = expand_scale(add = 2), coord_trans = sqrt_trans()), - c(1, (sqrt(100) + 2)^2) - ) -}) From 348b90b546ccd04ced9cc316cdfa48e5df2fe75f Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Fri, 21 Jun 2019 16:17:58 -0400 Subject: [PATCH 05/26] have coord polar and coord map use expansion --- R/coord-map.r | 7 +------ R/coord-polar.r | 12 ++++-------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/R/coord-map.r b/R/coord-map.r index 3af5ead0a7..4da260e516 100644 --- a/R/coord-map.r +++ b/R/coord-map.r @@ -181,12 +181,7 @@ CoordMap <- ggproto("CoordMap", Coord, for (n in c("x", "y")) { scale <- get(paste0("scale_", n)) limits <- self$limits[[n]] - - if (is.null(limits)) { - range <- scale$dimension(expand_default(scale)) - } else { - range <- range(scale$transform(limits)) - } + range <- scale$dimension(expand_default(scale), coord_limits = limits) ranges[[n]] <- range } diff --git a/R/coord-polar.r b/R/coord-polar.r index 85c0668e9c..314f85e22e 100644 --- a/R/coord-polar.r +++ b/R/coord-polar.r @@ -111,16 +111,12 @@ CoordPolar <- ggproto("CoordPolar", Coord, scale <- get(paste0("scale_", n)) limits <- self$limits[[n]] - if (is.null(limits)) { - if (self$theta == n) { - expand <- expand_default(scale, c(0, 0.5), c(0, 0)) - } else { - expand <- expand_default(scale, c(0, 0), c(0, 0)) - } - range <- scale$dimension(expand) + if (self$theta == n) { + expand <- expand_default(scale, c(0, 0.5), c(0, 0)) } else { - range <- range(scale_transform(scale, limits)) + expand <- expand_default(scale, c(0, 0), c(0, 0)) } + range <- scale$dimension(expand, coord_limits = limits) out <- scale$break_info(range) ret[[n]]$range <- out$range From c7310de829821e4181f434e42ec5b5b9a6157dbb Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Sat, 22 Jun 2019 21:45:14 -0400 Subject: [PATCH 06/26] add functions to expand continuous and discrete scale limits with transformations --- R/scale-expansion.r | 96 +++++++++++++++++++++++++++ tests/testthat/test-scale-expansion.r | 90 +++++++++++++++++++++++++ 2 files changed, 186 insertions(+) create mode 100644 tests/testthat/test-scale-expansion.r diff --git a/R/scale-expansion.r b/R/scale-expansion.r index e3b3f82407..fab1ef5f07 100644 --- a/R/scale-expansion.r +++ b/R/scale-expansion.r @@ -95,3 +95,99 @@ expand_default <- function(scale, discrete = expand_scale(add = 0.6), scale$expand %|W|% if (scale$is_discrete()) discrete else continuous } + +#' Expand limits in (possibly) transformed space +#' +#' These functions calculate the continuous range in coordinate space +#' and in scale space. Usually these can be calculated from +#' eachother using the coordinate system transformation, except +#' when transforming and expanding the scale limits results in values outside +#' the domain of the transformation (e.g., a lower limit of 0 with a square root +#' transformation). +#' +#' @param limits The initial scale limits, in scale-transformed space. +#' @param coord_limits The user-provided limits in scale-transformed space, +#' which may include one more more NA values, in which case those limits +#' will fall back to the `limits`. +#' @param expansion An expansion generated by [expand_scale()] or [expand_default()]. +#' @param trans The coordinate system transformation. +#' +#' @return A list with components `continuous_range`, which is the +#' expanded range in scale-transformed space, and `continuous_range_coord`, +#' which is the expanded range in coordinate-transformed space. +#' +#' @noRd +#' +expand_limits_continuous <- function(limits, coord_limits = c(NA, NA), expansion = expand_scale(0, 0)) { + expand_limits_continuous_trans(limits, coord_limits, expansion)$continuous_range +} + +expand_limits_discrete <- function(limits, coord_limits = c(NA, NA), expansion = expand_scale(0, 0), + range_continuous = NULL) { + limit_info <- expand_limits_discrete_trans( + limits, + coord_limits, + expansion, + range_continuous = range_continuous + ) + + limit_info$continuous_range +} + +expand_limits_continuous_trans <- function(limits, coord_limits = c(NA, NA), + expansion = expand_scale(0, 0), trans = identity_trans()) { + + # let non-NA coord_limits override the scale limits + limits <- ifelse(is.na(coord_limits), limits, coord_limits) + + # expand limits in coordinate space + continuous_range_coord <- trans$transform(limits) + continuous_range_coord <- expand_range4(continuous_range_coord, expansion) + final_scale_limits <- trans$inverse(continuous_range_coord) + + # if any non-finite values were introduced in the transformations, + # replace them with the original scale limits for the purposes of + # calculating breaks and minor breaks from the scale + continuous_range <- ifelse(is.finite(final_scale_limits), final_scale_limits, limits) + + list( + continuous_range_coord = continuous_range_coord, + continuous_range = continuous_range + ) +} + +expand_limits_discrete_trans <- function(limits, coord_limits = c(NA, NA), + expansion = expand_scale(0, 0), trans = identity_trans(), + range_continuous = NULL) { + + is_empty <- is.null(limits) && is.null(range_continuous) + is_only_continuous <- is.null(limits) + is_only_discrete <- is.null(range_continuous) + n_limits <- length(limits) + + if (is_empty) { + expand_limits_continuous_trans(c(0, 1), coord_limits, expansion, trans) + } else if (is_only_continuous) { + expand_limits_continuous_trans(range_continuous, coord_limits, expansion, trans) + } else if (is_only_discrete) { + expand_limits_continuous_trans(c(1, n_limits), coord_limits, expansion, trans) + } else { + # continuous and discrete + limit_info_discrete <- expand_limits_continuous_trans(c(1, n_limits), coord_limits, expansion, trans) + + # don't expand continuous range if there is also a discrete range + limit_info_continuous <- expand_limits_continuous_trans( + range_continuous, coord_limits, expand_scale(0, 0), trans + ) + + # prefer expanded discrete range, but allow continuous range to further expand the range + list( + continuous_range_coord = range( + c(limit_info_discrete$continuous_range_coord, limit_info_continuous$continuous_range_coord) + ), + continuous_range = range( + c(limit_info_discrete$continuous_range, limit_info_continuous$continuous_range) + ) + ) + } +} diff --git a/tests/testthat/test-scale-expansion.r b/tests/testthat/test-scale-expansion.r new file mode 100644 index 0000000000..aa1e1bf737 --- /dev/null +++ b/tests/testthat/test-scale-expansion.r @@ -0,0 +1,90 @@ + +# Expanding continuous scales ----------------------------------------- + +test_that("expand_limits_continuous() can override limits", { + expect_identical(expand_limits_continuous(c(1, 2), coord_limits = c(NA, NA)), c(1, 2)) + expect_identical(expand_limits_continuous(c(1, 2), coord_limits = c(NA, 3)), c(1, 3)) + expect_identical(expand_limits_continuous(c(1, 2), coord_limits = c(0, NA)), c(0, 2)) +}) + +test_that("expand_limits_continuous() expands limits", { + expect_identical(expand_limits_continuous(c(1, 2), expansion = expand_scale(add = 1)), c(0, 3)) +}) + +test_that("expand_limits_continuous() expands coord-supplied limits", { + expect_identical( + expand_limits_continuous(c(1, 2), coord_limits = c(0, 4), expansion = expand_scale(add = 1)), + c(-1, 5) + ) +}) + +test_that("expand_limits_continuous_trans() expands limits in coordinate space", { + limit_info <- expand_limits_continuous_trans( + c(1, 2), + expansion = expand_scale(add = 0.5), + trans = log10_trans() + ) + + expect_identical( + limit_info$continuous_range, + 10^(expand_range4(log10(c(1, 2)), expand_scale(add = 0.5))) + ) + + expect_identical( + limit_info$continuous_range_coord, + expand_range4(log10(c(1, 2)), expand_scale(add = 0.5)) + ) +}) + +test_that("introduced non-finite values fall back on scale limits", { + limit_info <- expand_limits_continuous_trans( + c(1, 100), + expansion = expand_scale(add = 2), + trans = sqrt_trans() + ) + + expect_identical(limit_info$continuous_range, c(1, (sqrt(100) + 2)^2)) + expect_identical(limit_info$continuous_range_coord, c(-1, sqrt(100) + 2)) +}) + +# Expanding discrete scales ----------------------------------------- + +test_that("expand_limits_discrete() can override limits with an empty range", { + expect_identical(expand_limits_discrete(NULL, coord_limits = c(-1, 8)), c(-1, 8)) +}) + +test_that("expand_limits_discrete() can override limits with a discrete range", { + expect_identical(expand_limits_discrete(c("one", "two"), coord_limits = c(NA, NA)), c(1, 2)) + expect_identical(expand_limits_discrete(c("one", "two"), coord_limits = c(NA, 3)), c(1, 3)) + expect_identical(expand_limits_discrete(c("one", "two"), coord_limits = c(3, NA)), c(3, 2)) +}) + +test_that("expand_limits_discrete() can override limits with a continuous range", { + expect_identical( + expand_limits_discrete(NULL, coord_limits = c(NA, NA), range_continuous = c(1, 2)), + c(1, 2) + ) + expect_identical( + expand_limits_discrete(NULL, coord_limits = c(NA, 3), range_continuous = c(1, 2)), + c(1, 3) + ) + expect_identical( + expand_limits_discrete(NULL, coord_limits = c(0, NA), range_continuous = c(1, 2)), + c(0, 2) + ) +}) + +test_that("expand_limits_discrete() can override limits with a both discrete and continuous ranges", { + expect_identical( + expand_limits_discrete(c("one", "two"), coord_limits = c(NA, NA), range_continuous = c(1, 2)), + c(1, 2) + ) + expect_identical( + expand_limits_discrete(c("one", "two"), coord_limits = c(NA, 3), range_continuous = c(1, 2)), + c(1, 3) + ) + expect_identical( + expand_limits_discrete(c("one", "two"), coord_limits = c(0, NA), range_continuous = c(1, 2)), + c(0, 2) + ) +}) From 18902525d49c35ee46a13a6ba279851ec268a67e Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Sat, 22 Jun 2019 22:08:00 -0400 Subject: [PATCH 07/26] apply new expansion strategy to coord_trans() --- R/coord-transform.r | 90 +++++++++++++-------------- man/coord_trans.Rd | 25 +++++--- tests/testthat/test-coord-transform.R | 25 ++++++++ 3 files changed, 85 insertions(+), 55 deletions(-) diff --git a/R/coord-transform.r b/R/coord-transform.r index 992ed31d45..066e1ee1dd 100644 --- a/R/coord-transform.r +++ b/R/coord-transform.r @@ -8,13 +8,9 @@ #' [scales::trans_new()] for list of transformations, and instructions #' on how to create your own. #' -#' @param x,y transformers for x and y axes -#' @param xtrans,ytrans Deprecated; use `x` and `y` instead. -#' @param limx,limy limits for x and y axes. (Named so for backward -#' compatibility) -#' @param clip Should drawing be clipped to the extent of the plot panel? A -#' setting of `"on"` (the default) means yes, and a setting of `"off"` -#' means no. For details, please see [`coord_cartesian()`]. +#' @inheritParams coord_cartesian +#' @param x,y Transformers for x and y axes or their names. +#' @param limx,limy These are deprecated - use `xlim` and `ylim` instead. #' @export #' @examples #' \donttest{ @@ -78,31 +74,25 @@ #' plot + coord_trans(x = "log10") #' plot + coord_trans(x = "sqrt") #' } -coord_trans <- function(x = "identity", y = "identity", limx = NULL, limy = NULL, clip = "on", - xtrans, ytrans) -{ - if (!missing(xtrans)) { - gg_dep("1.0.1", "`xtrans` arguments is deprecated; please use `x` instead.") - x <- xtrans +coord_trans <- function(x = "identity", y = "identity", xlim = NULL, ylim = NULL, + limx = NULL, limy = NULL, clip = "on", expand = TRUE) { + if (!missing(limx)) { + gg_dep("3.2.0", "`limx` argument is deprecated; please use `xlim` instead.") + xlim <- limx } - if (!missing(ytrans)) { - gg_dep("1.0.1", "`ytrans` arguments is deprecated; please use `y` instead.") - y <- ytrans + if (!missing(limy)) { + gg_dep("3.2.0", "`limy` argument is deprecated; please use `ylim` instead.") + ylim <- limy } - # @kohske - # Now limits are implemented. - # But for backward compatibility, xlim -> limx, ylim -> ylim - # Because there are many examples such as - # > coord_trans(x = "log10", y = "log10") - # Maybe this is changed. + # resolve transformers if (is.character(x)) x <- as.trans(x) if (is.character(y)) y <- as.trans(y) - ggproto(NULL, CoordTrans, trans = list(x = x, y = y), - limits = list(x = limx, y = limy), + limits = list(x = xlim, y = ylim), + expand = expand, clip = clip ) } @@ -147,8 +137,8 @@ CoordTrans <- ggproto("CoordTrans", Coord, setup_panel_params = function(self, scale_x, scale_y, params = list()) { c( - train_trans(scale_x, self$limits$x, self$trans$x, "x"), - train_trans(scale_y, self$limits$y, self$trans$y, "y") + train_trans(scale_x, self$limits$x, self$trans$x, "x", self$expand), + train_trans(scale_y, self$limits$y, self$trans$y, "y", self$expand) ) }, @@ -187,33 +177,37 @@ transform_value <- function(trans, value, range) { rescale(trans$transform(value), 0:1, range) } - -train_trans <- function(scale, limits, trans, name) { - # first, calculate the range that is the numerical limits in data space - - # expand defined by scale OR coord - # @kohske - # Expansion of data range sometimes go beyond domain, - # so in trans, expansion takes place at the final stage. - if (is.null(limits)) { - range <- scale$dimension() +train_trans <- function(scale, coord_limits, trans, name, expand = TRUE) { + expansion <- expand_default(scale, expand = expand) + scale_trans <- scale$trans %||% identity_trans() + coord_limits <- coord_limits %||% scale_trans$inverse(c(NA, NA)) + + if (scale$is_discrete()) { + continuous_ranges <- expand_limits_discrete_trans( + scale$get_limits(), + coord_limits, + expansion, + trans, + range_continuous = scale$range_c$range + ) } else { - range <- range(scale$transform(limits)) + # transform user-specified limits to scale transformed space + coord_limits <- scale$trans$transform(coord_limits) + continuous_ranges <- expand_limits_continuous_trans( + scale$get_limits(), + coord_limits, + expansion, + trans + ) } - # breaks on data space - out <- scale$break_info(range) - - # trans'd range - out$range <- trans$transform(out$range) + # calculate break information + out <- scale$break_info(continuous_ranges$continuous_range) - # expansion if limits are not specified - if (is.null(limits)) { - expand <- expand_default(scale) - out$range <- expand_range(out$range, expand[1], expand[2]) - } + # range in coord space has already been calculated + out$range <- continuous_ranges$continuous_range_coord - # major and minor values in plot space + # major and minor values in coordinate data out$major_source <- transform_value(trans, out$major_source, out$range) out$minor_source <- transform_value(trans, out$minor_source, out$range) diff --git a/man/coord_trans.Rd b/man/coord_trans.Rd index 56b7282b34..4f2acb70e5 100644 --- a/man/coord_trans.Rd +++ b/man/coord_trans.Rd @@ -4,20 +4,31 @@ \alias{coord_trans} \title{Transformed Cartesian coordinate system} \usage{ -coord_trans(x = "identity", y = "identity", limx = NULL, - limy = NULL, clip = "on", xtrans, ytrans) +coord_trans(x = "identity", y = "identity", xlim = NULL, + ylim = NULL, limx = NULL, limy = NULL, clip = "on", + expand = TRUE) } \arguments{ -\item{x, y}{transformers for x and y axes} +\item{x, y}{Transformers for x and y axes or their names.} -\item{limx, limy}{limits for x and y axes. (Named so for backward -compatibility)} +\item{xlim}{Limits for the x and y axes.} + +\item{ylim}{Limits for the x and y axes.} + +\item{limx, limy}{These are deprecated - use \code{xlim} and \code{ylim} instead.} \item{clip}{Should drawing be clipped to the extent of the plot panel? A setting of \code{"on"} (the default) means yes, and a setting of \code{"off"} -means no. For details, please see \code{\link[=coord_cartesian]{coord_cartesian()}}.} +means no. In most cases, the default of \code{"on"} should not be changed, +as setting \code{clip = "off"} can cause unexpected results. It allows +drawing of data points anywhere on the plot, including in the plot margins. If +limits are set via \code{xlim} and \code{ylim} and some data points fall outside those +limits, then those data points may show up in places such as the axes, the +legend, the plot title, or the plot margins.} -\item{xtrans, ytrans}{Deprecated; use \code{x} and \code{y} instead.} +\item{expand}{If \code{TRUE}, the default, adds a small expansion factor to +the limits to ensure that data and axes don't overlap. If \code{FALSE}, +limits are taken exactly from the data or \code{xlim}/\code{ylim}.} } \description{ \code{coord_trans} is different to scale transformations in that it occurs after diff --git a/tests/testthat/test-coord-transform.R b/tests/testthat/test-coord-transform.R index 5e089bc154..8a04eba970 100644 --- a/tests/testthat/test-coord-transform.R +++ b/tests/testthat/test-coord-transform.R @@ -20,3 +20,28 @@ test_that("no warnings are generated when original data has Inf values, but no n expect_silent(benchplot(p)) }) + +test_that("coord_trans() expands axes the identically coord_cartesian()", { + p <- ggplot(mpg, aes(class, hwy)) + geom_point() + built_cartesian <- ggplot_build(p + coord_cartesian()) + built_trans <- ggplot_build(p + coord_trans()) + + cartesian_params <- built_cartesian$layout$panel_params[[1]] + trans_params <- built_trans$layout$panel_params[[1]] + + expect_identical(cartesian_params$x.range, trans_params$x.range) + expect_identical(cartesian_params$y.range, trans_params$y.range) +}) + +test_that("coord_trans(expand = FALSE) expands axes the identically coord_cartesian(expand = FALSE)", { + p <- ggplot(mpg, aes(class, hwy)) + geom_point() + built_cartesian <- ggplot_build(p + coord_cartesian(expand = FALSE)) + built_trans <- ggplot_build(p + coord_trans(expand = FALSE)) + + cartesian_params <- built_cartesian$layout$panel_params[[1]] + trans_params <- built_trans$layout$panel_params[[1]] + + expect_identical(cartesian_params$x.range, trans_params$x.range) + expect_identical(cartesian_params$y.range, trans_params$y.range) +}) + From 5d5b6791685dbac254b900b8f2bc9aade6cb4042 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Sat, 22 Jun 2019 22:31:52 -0400 Subject: [PATCH 08/26] have all expansion take place using functions in scale-expansion.r --- R/scale-.r | 6 ++---- R/scale-discrete-.r | 20 +------------------- R/scale-expansion.r | 4 ++-- 3 files changed, 5 insertions(+), 25 deletions(-) diff --git a/R/scale-.r b/R/scale-.r index 6c374101c5..7db1f8e788 100644 --- a/R/scale-.r +++ b/R/scale-.r @@ -491,9 +491,7 @@ ScaleContinuous <- ggproto("ScaleContinuous", Scale, coord_limits = NULL) { coord_limits <- coord_limits %||% self$trans$inverse(c(NA, NA)) coord_limits_scale <- self$trans$transform(coord_limits) - limits <- ifelse(is.na(coord_limits_scale), limits, coord_limits_scale) - - expand_range4(limits, expand) + expand_limits_continuous(limits, coord_limits_scale, expand) }, get_breaks = function(self, limits = self$get_limits()) { @@ -694,7 +692,7 @@ ScaleDiscrete <- ggproto("ScaleDiscrete", Scale, }, dimension = function(self, expand = expand_scale(0, 0), limits = self$get_limits()) { - expand_range4(c(1, length(limits)), expand) + expand_limits_discrete(limits, expansion = expand) }, get_breaks = function(self, limits = self$get_limits()) { diff --git a/R/scale-discrete-.r b/R/scale-discrete-.r index ae04d88e67..760a6305df 100644 --- a/R/scale-discrete-.r +++ b/R/scale-discrete-.r @@ -108,25 +108,7 @@ ScaleDiscretePosition <- ggproto("ScaleDiscretePosition", ScaleDiscrete, dimension = function(self, expand = expand_scale(0, 0), limits = self$get_limits(), coord_limits = NULL) { coord_limits <- coord_limits %||% c(NA_real_, NA_real_) - - c_range <- self$range_c$range - d_range <- limits - - if (self$is_empty()) { - c(0, 1) - } else if (is.null(self$range$range)) { # only continuous - c_range <- ifelse(is.na(coord_limits), c_range, coord_limits) - expand_range4(c_range, expand) - } else if (is.null(c_range) || any(!is.na(coord_limits))) { # only discrete or coord-limited - c_range <- ifelse(is.na(coord_limits), c(1, length(d_range)), coord_limits) - expand_range4(c_range, expand) - } else { # both - discrete_c_range <- ifelse(is.na(coord_limits), c(1, length(d_range)), coord_limits) - range( - c_range, - expand_range4(discrete_c_range, expand) - ) - } + expand_limits_discrete(self$get_limits(), coord_limits, expand, range_continuous = self$range_c$range) }, clone = function(self) { diff --git a/R/scale-expansion.r b/R/scale-expansion.r index fab1ef5f07..e91401305f 100644 --- a/R/scale-expansion.r +++ b/R/scale-expansion.r @@ -160,10 +160,10 @@ expand_limits_discrete_trans <- function(limits, coord_limits = c(NA, NA), expansion = expand_scale(0, 0), trans = identity_trans(), range_continuous = NULL) { + n_limits <- length(limits) is_empty <- is.null(limits) && is.null(range_continuous) - is_only_continuous <- is.null(limits) + is_only_continuous <- n_limits == 0 is_only_discrete <- is.null(range_continuous) - n_limits <- length(limits) if (is_empty) { expand_limits_continuous_trans(c(0, 1), coord_limits, expansion, trans) From 1c4adcdded80f4138e241b6924d02ea62c343a0c Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Sun, 23 Jun 2019 15:05:42 -0400 Subject: [PATCH 09/26] get second axes to work with coord_trans() --- R/axis-secondary.R | 50 ++- R/coord-polar.r | 4 +- R/coord-transform.r | 14 +- .../coord-trans/basic-coord-trans-plot.svg | 285 ++++++++++++++++ .../coord-trans/sec-axis-with-coord-trans.svg | 311 ++++++++++++++++++ tests/testthat/test-coord-transform.R | 71 +++- tests/testthat/test-sec-axis.R | 42 ++- 7 files changed, 747 insertions(+), 30 deletions(-) create mode 100644 tests/figs/coord-trans/basic-coord-trans-plot.svg create mode 100644 tests/figs/coord-trans/sec-axis-with-coord-trans.svg diff --git a/R/axis-secondary.R b/R/axis-secondary.R index 87a6893dba..e7de5f44a6 100644 --- a/R/axis-secondary.R +++ b/R/axis-secondary.R @@ -187,21 +187,49 @@ AxisSecondary <- ggproto("AxisSecondary", NULL, # patch for date and datetime scales just to maintain functionality # works only for linear secondary transforms that respect the time or date transform - if (scale$trans$name %in% c("date", "time")){ + if (scale$trans$name %in% c("date", "time")) { temp_scale <- self$create_scale(new_range, trans = scale$trans) range_info <- temp_scale$break_info() - names(range_info) <- paste0("sec.", names(range_info)) - return(range_info) - } + old_val_trans <- rescale(range_info$major, from = c(0, 1), to = range) + old_val_minor_trans <- rescale(range_info$minor, from = c(0, 1), to = range) + } else { + temp_scale <- self$create_scale(new_range) + range_info <- temp_scale$break_info() - temp_scale <- self$create_scale(new_range) - range_info <- temp_scale$break_info() + # Map the break values back to their correct position on the primary scale + old_val <- lapply(range_info$major_source, function(x) which.min(abs(full_range - x))) + old_val <- old_range[unlist(old_val)] + old_val_trans <- scale$trans$transform(old_val) + + old_val_minor <- lapply(range_info$minor_source, function(x) which.min(abs(full_range - x))) + old_val_minor <- old_range[unlist(old_val_minor)] + old_val_minor_trans <- scale$trans$transform(old_val_minor) + + # rescale values from 0 to 1 + range_info$major[] <- round( + rescale( + scale$map(old_val_trans, range(old_val_trans)), + from = range + ), + digits = 3 + ) + + range_info$minor[] <- round( + rescale( + scale$map(old_val_minor_trans, range(old_val_minor_trans)), + from = range + ), + digits = 3 + ) + } - # Map the break values back to their correct position on the primary scale - old_val <- lapply(range_info$major_source, function(x) which.min(abs(full_range - x))) - old_val <- old_range[unlist(old_val)] - old_val_trans <- scale$trans$transform(old_val) - range_info$major[] <- round(rescale(scale$map(old_val_trans, range(old_val_trans)), from = range), digits = 3) + # The _source values should be in (primary) scale_transformed space, + # so that the coord doesn't have to know about the secondary scale transformation + # when drawing the axis. The values in user space are useful for testing. + range_info$major_source_user <- range_info$major_source + range_info$minor_source_user <- range_info$minor_source + range_info$major_source[] <- old_val_trans + range_info$minor_source[] <- old_val_minor_trans names(range_info) <- paste0("sec.", names(range_info)) range_info diff --git a/R/coord-polar.r b/R/coord-polar.r index 314f85e22e..ac3eed0409 100644 --- a/R/coord-polar.r +++ b/R/coord-polar.r @@ -124,8 +124,8 @@ CoordPolar <- ggproto("CoordPolar", Coord, ret[[n]]$minor <- out$minor_source ret[[n]]$labels <- out$labels ret[[n]]$sec.range <- out$sec.range - ret[[n]]$sec.major <- out$sec.major_source - ret[[n]]$sec.minor <- out$sec.minor_source + ret[[n]]$sec.major <- out$sec.major_source_user + ret[[n]]$sec.minor <- out$sec.minor_source_user ret[[n]]$sec.labels <- out$sec.labels } diff --git a/R/coord-transform.r b/R/coord-transform.r index 066e1ee1dd..5b52be6d53 100644 --- a/R/coord-transform.r +++ b/R/coord-transform.r @@ -205,15 +205,23 @@ train_trans <- function(scale, coord_limits, trans, name, expand = TRUE) { out <- scale$break_info(continuous_ranges$continuous_range) # range in coord space has already been calculated - out$range <- continuous_ranges$continuous_range_coord + # needs to be in increasing order for transform_value() to work + out$range <- range(continuous_ranges$continuous_range_coord) # major and minor values in coordinate data out$major_source <- transform_value(trans, out$major_source, out$range) out$minor_source <- transform_value(trans, out$minor_source, out$range) + out$sec.major_source <- transform_value(trans, out$sec.major_source, out$range) + out$sec.minor_source <- transform_value(trans, out$sec.minor_source, out$range) out <- list( - range = out$range, labels = out$labels, - major = out$major_source, minor = out$minor_source + range = out$range, + labels = out$labels, + major = out$major_source, + minor = out$minor_source, + sec.labels = out$sec.labels, + sec.major = out$sec.major_source, + sec.minor = out$sec.minor_source ) names(out) <- paste(name, names(out), sep = ".") out diff --git a/tests/figs/coord-trans/basic-coord-trans-plot.svg b/tests/figs/coord-trans/basic-coord-trans-plot.svg new file mode 100644 index 0000000000..2465761e51 --- /dev/null +++ b/tests/figs/coord-trans/basic-coord-trans-plot.svg @@ -0,0 +1,285 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +20 +30 +40 + + + + + + + + + + +2seater +compact +midsize +minivan +pickup +subcompact +suv +class +hwy +basic coord_trans() plot + diff --git a/tests/figs/coord-trans/sec-axis-with-coord-trans.svg b/tests/figs/coord-trans/sec-axis-with-coord-trans.svg new file mode 100644 index 0000000000..55015fbeff --- /dev/null +++ b/tests/figs/coord-trans/sec-axis-with-coord-trans.svg @@ -0,0 +1,311 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +10 +15 +20 +25 +30 +35 + + + + + + +11.31371 +16.00000 +22.62742 +32.00000 +45.25483 + + + + + + + + + + +3.5 +4.0 +4.5 +5.0 +5.5 + + + + + + +10 +15 +20 +25 +30 +35 +cty +cty +hwy +log2(hwy) +sec_axis with coord_trans() + diff --git a/tests/testthat/test-coord-transform.R b/tests/testthat/test-coord-transform.R index 8a04eba970..247e04d14b 100644 --- a/tests/testthat/test-coord-transform.R +++ b/tests/testthat/test-coord-transform.R @@ -21,7 +21,7 @@ test_that("no warnings are generated when original data has Inf values, but no n expect_silent(benchplot(p)) }) -test_that("coord_trans() expands axes the identically coord_cartesian()", { +test_that("coord_trans() expands axes identically to coord_cartesian()", { p <- ggplot(mpg, aes(class, hwy)) + geom_point() built_cartesian <- ggplot_build(p + coord_cartesian()) built_trans <- ggplot_build(p + coord_trans()) @@ -33,7 +33,7 @@ test_that("coord_trans() expands axes the identically coord_cartesian()", { expect_identical(cartesian_params$y.range, trans_params$y.range) }) -test_that("coord_trans(expand = FALSE) expands axes the identically coord_cartesian(expand = FALSE)", { +test_that("coord_trans(expand = FALSE) expands axes identically to coord_cartesian(expand = FALSE)", { p <- ggplot(mpg, aes(class, hwy)) + geom_point() built_cartesian <- ggplot_build(p + coord_cartesian(expand = FALSE)) built_trans <- ggplot_build(p + coord_trans(expand = FALSE)) @@ -45,3 +45,70 @@ test_that("coord_trans(expand = FALSE) expands axes the identically coord_cartes expect_identical(cartesian_params$y.range, trans_params$y.range) }) +test_that("coord_trans(y = 'log10') expands the x axis identically to scale_y_log10()", { + p <- ggplot(mpg, aes(class, hwy)) + geom_point() + built_cartesian <- ggplot_build(p + scale_y_log10()) + built_trans <- ggplot_build(p + coord_trans(y = "log10")) + + cartesian_params <- built_cartesian$layout$panel_params[[1]] + trans_params <- built_trans$layout$panel_params[[1]] + + expect_identical(cartesian_params$x.range, trans_params$x.range) + expect_identical(cartesian_params$y.range, trans_params$y.range) +}) + +test_that("coord_trans() expands axes outside the domain of the axis trans", { + # sqrt_trans() has a lower limit of 0 + df <- data_frame(x = 1, y = c(0, 1, 2)) + p <- ggplot(df, aes(x, y)) + geom_point() + built_cartesian <- ggplot_build(p + scale_y_sqrt()) + built_trans <- ggplot_build(p + coord_trans(y = "sqrt")) + + cartesian_params <- built_cartesian$layout$panel_params[[1]] + trans_params <- built_trans$layout$panel_params[[1]] + + expect_identical(cartesian_params$x.range, trans_params$x.range) + expect_identical(cartesian_params$y.range, trans_params$y.range) +}) + +test_that("coord_trans() can use the reverse transformation for continuous axes", { + df <- data_frame(x = c("1-one", "2-two", "3-three"), y = c(20, 30, 40)) + + ggplot(df, aes(x, y)) + + geom_point() + + coord_trans(x = "reverse") + + cartesian_params <- built_cartesian$layout$panel_params[[1]] + trans_params <- built_trans$layout$panel_params[[1]] + + expect_identical(cartesian_params$x.range, trans_params$x.range) + expect_identical(cartesian_params$y.range, trans_params$y.range) +}) + + +test_that("basic coord_trans() plot displays both continuous and discrete axes", { + expect_doppelganger( + "basic coord_trans() plot", + ggplot(mpg, aes(class, hwy)) + + geom_point() + + coord_trans(y = "log10") + ) +}) + +test_that("second axes display in coord_trans()", { + expect_doppelganger( + "sec_axis with coord_trans()", + ggplot(mpg, aes(cty, hwy)) + + geom_point() + + scale_y_continuous( + sec.axis = sec_axis( + trans = ~log2(.), + breaks = c(3.5, 4, 4.5, 5, 5.5), + name = "log2(hwy)" + ), + breaks = 2^c(3.5, 4, 4.5, 5, 5.5) + ) + + scale_x_continuous(sec.axis = dup_axis()) + + coord_trans(y = "log2") + ) +}) diff --git a/tests/testthat/test-sec-axis.R b/tests/testthat/test-sec-axis.R index d4b4e45cad..221d1468c9 100644 --- a/tests/testthat/test-sec-axis.R +++ b/tests/testthat/test-sec-axis.R @@ -16,8 +16,15 @@ test_that("dup_axis() works", { scale <- layer_scales(p)$x expect_equal(scale$sec_name(), scale$name) breaks <- scale$break_info() - expect_equal(breaks$minor, breaks$sec.minor) - expect_equal(breaks$major_source, breaks$sec.major_source) + expect_equal(breaks$minor_source, breaks$sec.minor_source_user) + expect_equal(breaks$major_source, breaks$sec.major_source_user) + + # these aren't exactly equal because the sec_axis trans is based on a + # (default) 1000-point approximation + expect_true(all(abs(breaks$major_source - round(breaks$sec.major_source) <= 1))) + expect_true(all(abs(breaks$minor_source - round(breaks$sec.minor_source) <= 1))) + expect_equal(round(breaks$major, 3), round(breaks$major, 3)) + expect_equal(round(breaks$minor, 3), round(breaks$minor, 3)) }) test_that("sec_axis() works with subtraction", { @@ -29,8 +36,15 @@ test_that("sec_axis() works with subtraction", { scale <- layer_scales(p)$y expect_equal(scale$sec_name(), scale$name) breaks <- scale$break_info() - expect_equal(breaks$minor, breaks$sec.minor) - expect_equal(breaks$major_source, breaks$sec.major_source) + expect_equal(breaks$minor_source, breaks$sec.minor_source_user) + expect_equal(breaks$major_source, breaks$sec.major_source_user) + + # these aren't exactly equal because the sec_axis trans is based on a + # (default) 1000-point approximation + expect_true(all(abs(breaks$major_source - round(breaks$sec.major_source) <= 1))) + expect_true(all(abs(breaks$minor_source - round(breaks$sec.minor_source) <= 1))) + expect_equal(round(breaks$major, 3), round(breaks$major, 3)) + expect_equal(round(breaks$minor, 3), round(breaks$minor, 3)) }) test_that("sex axis works with division (#1804)", { @@ -59,7 +73,9 @@ test_that("sec_axis() breaks work for log-transformed scales", { breaks <- scale$break_info() # test value - expect_equal(breaks$major_source, log10(breaks$sec.major_source)) + expect_equal(breaks$major_source, log10(breaks$sec.major_source_user)) + expect_equal(round(breaks$major_source, 2), round(breaks$sec.major_source, 2)) + # test position expect_equal(breaks$major, round(breaks$sec.major, 1)) @@ -72,7 +88,9 @@ test_that("sec_axis() breaks work for log-transformed scales", { breaks <- scale$break_info() # test value - expect_equal(breaks$major_source, log10(breaks$sec.major_source) - 2) + expect_equal(breaks$major_source, log10(breaks$sec.major_source_user) - 2) + expect_equal(breaks$major_source, round(breaks$sec.major_source, 2)) + # test position expect_equal(breaks$major, round(breaks$sec.major, 1)) @@ -87,7 +105,7 @@ test_that("sec_axis() breaks work for log-transformed scales", { breaks <- scale$break_info() expect_equal(breaks$major_source, log(custom_breaks, base = 10)) - expect_equal(log_breaks()(df$y) * 100, breaks$sec.major_source) + expect_equal(log_breaks()(df$y) * 100, breaks$sec.major_source_user) }) test_that("custom breaks work", { @@ -103,7 +121,7 @@ test_that("custom breaks work", { ) scale <- layer_scales(p)$x breaks <- scale$break_info() - expect_equal(custom_breaks, breaks$sec.major_source) + expect_equal(custom_breaks, breaks$sec.major_source_user) }) test_that("sec axis works with skewed transform", { @@ -149,7 +167,7 @@ test_that("sec axis works with tidy eval", { breaks <- scale$break_info() # test transform - expect_equal(breaks$major_source / 10, breaks$sec.major_source) + expect_equal(breaks$major_source / 10, breaks$sec.major_source_user) # test positioning expect_equal(round(breaks$major, 2), round(breaks$sec.major, 2)) }) @@ -246,7 +264,7 @@ test_that("sec_axis works with date/time/datetime scales", { scale_x_datetime(sec.axis = dup_axis()) scale <- layer_scales(dt)$x breaks <- scale$break_info() - expect_equal(breaks$major_source, breaks$sec.major_source) + expect_equal(breaks$major_source, breaks$sec.major_source_user) # datetime scale dt <- ggplot(df, aes(date, price)) + @@ -254,7 +272,7 @@ test_that("sec_axis works with date/time/datetime scales", { scale_x_date(sec.axis = dup_axis()) scale <- layer_scales(dt)$x breaks <- scale$break_info() - expect_equal(breaks$major_source, breaks$sec.major_source) + expect_equal(breaks$major_source, breaks$sec.major_source_user) # sec_axis dt <- ggplot(df, aes(dx, price)) + @@ -270,7 +288,7 @@ test_that("sec_axis works with date/time/datetime scales", { expect_equal( as.numeric(breaks$major_source) + 12 * 60 * 60, - as.numeric(breaks$sec.major_source) + as.numeric(breaks$sec.major_source_user) ) # visual test, datetime scales, reprex #1936 From dfd89deadb18a177fc09450b58a75f6c7da82d0f Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Sun, 23 Jun 2019 15:29:41 -0400 Subject: [PATCH 10/26] ensure that the reverse/reciprocal transformations work with coord_trans() --- R/scale-expansion.r | 10 +++++++++- tests/testthat/test-coord-transform.R | 23 ++++++++++++++++------- tests/testthat/test-scale-expansion.r | 11 +++++++++++ 3 files changed, 36 insertions(+), 8 deletions(-) diff --git a/R/scale-expansion.r b/R/scale-expansion.r index e91401305f..90aac7859d 100644 --- a/R/scale-expansion.r +++ b/R/scale-expansion.r @@ -142,7 +142,15 @@ expand_limits_continuous_trans <- function(limits, coord_limits = c(NA, NA), # expand limits in coordinate space continuous_range_coord <- trans$transform(limits) - continuous_range_coord <- expand_range4(continuous_range_coord, expansion) + + # range expansion expects values in increasing order, which may not be true + # for reciprocal/reverse transformations + if (all(is.finite(continuous_range_coord)) && diff(continuous_range_coord) < 0) { + continuous_range_coord <- rev(expand_range4(rev(continuous_range_coord), expansion)) + } else { + continuous_range_coord <- expand_range4(continuous_range_coord, expansion) + } + final_scale_limits <- trans$inverse(continuous_range_coord) # if any non-finite values were introduced in the transformations, diff --git a/tests/testthat/test-coord-transform.R b/tests/testthat/test-coord-transform.R index 247e04d14b..1b2ecfcc11 100644 --- a/tests/testthat/test-coord-transform.R +++ b/tests/testthat/test-coord-transform.R @@ -53,7 +53,6 @@ test_that("coord_trans(y = 'log10') expands the x axis identically to scale_y_lo cartesian_params <- built_cartesian$layout$panel_params[[1]] trans_params <- built_trans$layout$panel_params[[1]] - expect_identical(cartesian_params$x.range, trans_params$x.range) expect_identical(cartesian_params$y.range, trans_params$y.range) }) @@ -67,24 +66,34 @@ test_that("coord_trans() expands axes outside the domain of the axis trans", { cartesian_params <- built_cartesian$layout$panel_params[[1]] trans_params <- built_trans$layout$panel_params[[1]] - expect_identical(cartesian_params$x.range, trans_params$x.range) expect_identical(cartesian_params$y.range, trans_params$y.range) }) -test_that("coord_trans() can use the reverse transformation for continuous axes", { +test_that("coord_trans() works with the reverse transformation", { df <- data_frame(x = c("1-one", "2-two", "3-three"), y = c(20, 30, 40)) - ggplot(df, aes(x, y)) + - geom_point() + - coord_trans(x = "reverse") + p <- ggplot(df, aes(x, y)) + geom_point() + built_cartesian <- ggplot_build(p + scale_y_reverse()) + built_trans <- ggplot_build(p + coord_trans(y = "reverse")) cartesian_params <- built_cartesian$layout$panel_params[[1]] trans_params <- built_trans$layout$panel_params[[1]] - expect_identical(cartesian_params$x.range, trans_params$x.range) expect_identical(cartesian_params$y.range, trans_params$y.range) }) +test_that("coord_trans() can reverse discrete axes", { + df <- data_frame(x = c("1-one", "2-two", "3-three"), y = c(20, 30, 40)) + + p <- ggplot(df, aes(x, y)) + geom_point() + built_cartesian <- ggplot_build(p) + built_trans <- ggplot_build(p + coord_trans(x = "reverse")) + + cartesian_params <- built_cartesian$layout$panel_params[[1]] + trans_params <- built_trans$layout$panel_params[[1]] + + expect_identical(cartesian_params$x.range, -rev(trans_params$x.range)) +}) test_that("basic coord_trans() plot displays both continuous and discrete axes", { expect_doppelganger( diff --git a/tests/testthat/test-scale-expansion.r b/tests/testthat/test-scale-expansion.r index aa1e1bf737..4501a9065b 100644 --- a/tests/testthat/test-scale-expansion.r +++ b/tests/testthat/test-scale-expansion.r @@ -88,3 +88,14 @@ test_that("expand_limits_discrete() can override limits with a both discrete and c(0, 2) ) }) + +test_that("expand_limits_continuous_trans() works with inverted transformations", { + limit_info <- expand_limits_continuous_trans( + c(1, 2), + expansion = expand_scale(add = 1), + trans = reverse_trans() + ) + + expect_identical(limit_info$continuous_range, c(0, 3)) + expect_identical(limit_info$continuous_range_coord, c(0, -3)) +}) From be2a229bbea49675995f7dcedf17cce2e70c2f40 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Sun, 23 Jun 2019 16:16:28 -0400 Subject: [PATCH 11/26] add news bullets --- NEWS.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/NEWS.md b/NEWS.md index c8bd1caa21..a8a6d8b91c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,23 @@ # ggplot2 (development version) +* `coord_trans()` now draws second axes and accepts `xlim`, `ylim`, + and `expand` arguments to bring it up to feature parity with + `coord_cartesian()` (@paleolimbot, #2990). + +* `coord_trans()` now calculates breaks using the expanded range + (previously these were calculated using the unexpanded range, + which resulted in differences between plots made with `coord_trans()` + and those made with `coord_cartesian()`). The expansion for discrete axes + in `coord_trans()` was also updated such that it behaves identically + to that in `coord_cartesian()` (@paleolimbot, #3338). + +* All `coord_*()` functions with `xlim` and `ylim` arguments now accept + vectors with `NA` as a placeholder for the minimum or maximum value + (e.g., `ylim = c(0, NA)` would zoom the y-axis from 0 to the + maximum value observed in the data). This mimics the behaviour + of the `limits` argument in continuous scale functions + (@paleolimbot, #2907). + * `geom_abline()`, `geom_hline()`, and `geom_vline()` now issue more informative warnings when supplied with set aesthetics (i.e., `slope`, `intercept`, `yintercept`, and/or `xintercept`) From 6c79a04da728fe2de5472852327bd5312eb31a46 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Sun, 23 Jun 2019 17:16:56 -0400 Subject: [PATCH 12/26] remove all coord limiting/expansion code from scale functions --- R/coord-cartesian-.r | 2 +- R/coord-map.r | 2 +- R/coord-polar.r | 6 +++--- R/coord-sf.R | 12 ++++++++++-- R/scale-.r | 7 ++----- R/scale-discrete-.r | 6 ++---- R/scale-expansion.r | 21 ++++++++++++++++++++- 7 files changed, 39 insertions(+), 17 deletions(-) diff --git a/R/coord-cartesian-.r b/R/coord-cartesian-.r index e356ace11b..2b7af37a4f 100644 --- a/R/coord-cartesian-.r +++ b/R/coord-cartesian-.r @@ -139,7 +139,7 @@ CoordCartesian <- ggproto("CoordCartesian", Coord, view_scales_from_scale <- function(scale, coord_limits = NULL, expand = TRUE) { expansion <- expand_default(scale, expand = expand) limits <- scale$get_limits() - continuous_range <- scale$dimension(expansion, coord_limits = coord_limits) + continuous_range <- expand_limits_scale(scale, expansion, limits, coord_limits = coord_limits) aesthetic <- scale$aesthetics[1] view_scales <- list( diff --git a/R/coord-map.r b/R/coord-map.r index 4da260e516..093894b3c1 100644 --- a/R/coord-map.r +++ b/R/coord-map.r @@ -181,7 +181,7 @@ CoordMap <- ggproto("CoordMap", Coord, for (n in c("x", "y")) { scale <- get(paste0("scale_", n)) limits <- self$limits[[n]] - range <- scale$dimension(expand_default(scale), coord_limits = limits) + range <- expand_limits_scale(scale, expand_default(scale), coord_limits = limits) ranges[[n]] <- range } diff --git a/R/coord-polar.r b/R/coord-polar.r index ac3eed0409..5c78079957 100644 --- a/R/coord-polar.r +++ b/R/coord-polar.r @@ -112,11 +112,11 @@ CoordPolar <- ggproto("CoordPolar", Coord, limits <- self$limits[[n]] if (self$theta == n) { - expand <- expand_default(scale, c(0, 0.5), c(0, 0)) + expansion <- expand_default(scale, c(0, 0.5), c(0, 0)) } else { - expand <- expand_default(scale, c(0, 0), c(0, 0)) + expansion <- expand_default(scale, c(0, 0), c(0, 0)) } - range <- scale$dimension(expand, coord_limits = limits) + range <- expand_limits_scale(scale, expansion, coord_limits = limits) out <- scale$break_info(range) ret[[n]]$range <- out$range diff --git a/R/coord-sf.R b/R/coord-sf.R index 7ea1905563..567311a182 100644 --- a/R/coord-sf.R +++ b/R/coord-sf.R @@ -127,8 +127,16 @@ CoordSf <- ggproto("CoordSf", CoordCartesian, setup_panel_params = function(self, scale_x, scale_y, params = list()) { # Bounding box of the data - x_range <- scale_x$dimension(expand_default(scale_x, expand = self$expand), coord_limits = self$limits$x) - y_range <- scale_y$dimension(expand_default(scale_y, expand = self$expand), coord_limits = self$limits$y) + x_range <- expand_limits_scale( + scale_x, + expand_default(scale_x, expand = self$expand), + coord_limits = self$limits$x + ) + y_range <- expand_limits_scale( + scale_y, + expand_default(scale_y, expand = self$expand), + coord_limits = self$limits$y + ) bbox <- c( x_range[1], y_range[1], x_range[2], y_range[2] diff --git a/R/scale-.r b/R/scale-.r index 7db1f8e788..fd9b8be360 100644 --- a/R/scale-.r +++ b/R/scale-.r @@ -487,11 +487,8 @@ ScaleContinuous <- ggproto("ScaleContinuous", Scale, self$rescaler(x, from = range) }, - dimension = function(self, expand = expand_scale(0, 0), limits = self$get_limits(), - coord_limits = NULL) { - coord_limits <- coord_limits %||% self$trans$inverse(c(NA, NA)) - coord_limits_scale <- self$trans$transform(coord_limits) - expand_limits_continuous(limits, coord_limits_scale, expand) + dimension = function(self, expand = expand_scale(0, 0), limits = self$get_limits()) { + expand_limits_scale(self, expand, limits) }, get_breaks = function(self, limits = self$get_limits()) { diff --git a/R/scale-discrete-.r b/R/scale-discrete-.r index 760a6305df..e2307c17d3 100644 --- a/R/scale-discrete-.r +++ b/R/scale-discrete-.r @@ -105,10 +105,8 @@ ScaleDiscretePosition <- ggproto("ScaleDiscretePosition", ScaleDiscrete, rescale(self$map(x, limits = limits), from = range) }, - dimension = function(self, expand = expand_scale(0, 0), limits = self$get_limits(), - coord_limits = NULL) { - coord_limits <- coord_limits %||% c(NA_real_, NA_real_) - expand_limits_discrete(self$get_limits(), coord_limits, expand, range_continuous = self$range_c$range) + dimension = function(self, expand = expand_scale(0, 0), limits = self$get_limits()) { + expand_limits_scale(self, expand, limits) }, clone = function(self) { diff --git a/R/scale-expansion.r b/R/scale-expansion.r index 90aac7859d..6bf1673608 100644 --- a/R/scale-expansion.r +++ b/R/scale-expansion.r @@ -105,6 +105,7 @@ expand_default <- function(scale, discrete = expand_scale(add = 0.6), #' the domain of the transformation (e.g., a lower limit of 0 with a square root #' transformation). #' +#' @param scale A position scale (see [scale_x_continuous()] and [scale_x_discrete()]) #' @param limits The initial scale limits, in scale-transformed space. #' @param coord_limits The user-provided limits in scale-transformed space, #' which may include one more more NA values, in which case those limits @@ -117,7 +118,25 @@ expand_default <- function(scale, discrete = expand_scale(add = 0.6), #' which is the expanded range in coordinate-transformed space. #' #' @noRd -#' +expand_limits_scale <- function(scale, expansion = expand_scale(0, 0), limits = waiver(), + coord_limits = NULL) { + limits <- limits %|W|% scale$get_limits() + + if (scale$is_discrete()) { + coord_limits <- coord_limits %||% c(NA_real_, NA_real_) + expand_limits_discrete( + limits, + coord_limits, + expansion, + range_continuous = scale$range_c$range + ) + } else { + coord_limits <- coord_limits %||% scale$trans$inverse(c(NA_real_, NA_real_)) + coord_limits_scale <- scale$trans$transform(coord_limits) + expand_limits_continuous(limits, coord_limits_scale, expansion) + } +} + expand_limits_continuous <- function(limits, coord_limits = c(NA, NA), expansion = expand_scale(0, 0)) { expand_limits_continuous_trans(limits, coord_limits, expansion)$continuous_range } From 50414279344bc166d67a0b3968f3085392efb422 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Sun, 23 Jun 2019 17:25:01 -0400 Subject: [PATCH 13/26] change argument order: should always be limits, expansion, coord_limits --- R/coord-transform.r | 4 ++-- R/scale-expansion.r | 33 ++++++++++++++++++--------------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/R/coord-transform.r b/R/coord-transform.r index 5b52be6d53..ee0a28d5dc 100644 --- a/R/coord-transform.r +++ b/R/coord-transform.r @@ -185,8 +185,8 @@ train_trans <- function(scale, coord_limits, trans, name, expand = TRUE) { if (scale$is_discrete()) { continuous_ranges <- expand_limits_discrete_trans( scale$get_limits(), - coord_limits, expansion, + coord_limits, trans, range_continuous = scale$range_c$range ) @@ -195,8 +195,8 @@ train_trans <- function(scale, coord_limits, trans, name, expand = TRUE) { coord_limits <- scale$trans$transform(coord_limits) continuous_ranges <- expand_limits_continuous_trans( scale$get_limits(), - coord_limits, expansion, + coord_limits, trans ) } diff --git a/R/scale-expansion.r b/R/scale-expansion.r index 6bf1673608..ebb7d02a89 100644 --- a/R/scale-expansion.r +++ b/R/scale-expansion.r @@ -118,6 +118,7 @@ expand_default <- function(scale, discrete = expand_scale(add = 0.6), #' which is the expanded range in coordinate-transformed space. #' #' @noRd +#' expand_limits_scale <- function(scale, expansion = expand_scale(0, 0), limits = waiver(), coord_limits = NULL) { limits <- limits %|W|% scale$get_limits() @@ -126,35 +127,37 @@ expand_limits_scale <- function(scale, expansion = expand_scale(0, 0), limits = coord_limits <- coord_limits %||% c(NA_real_, NA_real_) expand_limits_discrete( limits, - coord_limits, expansion, + coord_limits, range_continuous = scale$range_c$range ) } else { + # using the inverse transform to resolve the NA value is needed for date/datetime/time + # scales, which refuse to transform objects of the incorrect type coord_limits <- coord_limits %||% scale$trans$inverse(c(NA_real_, NA_real_)) coord_limits_scale <- scale$trans$transform(coord_limits) - expand_limits_continuous(limits, coord_limits_scale, expansion) + expand_limits_continuous(limits, expansion, coord_limits_scale) } } -expand_limits_continuous <- function(limits, coord_limits = c(NA, NA), expansion = expand_scale(0, 0)) { - expand_limits_continuous_trans(limits, coord_limits, expansion)$continuous_range +expand_limits_continuous <- function(limits, expansion = expand_scale(0, 0), coord_limits = c(NA, NA)) { + expand_limits_continuous_trans(limits, expansion, coord_limits)$continuous_range } -expand_limits_discrete <- function(limits, coord_limits = c(NA, NA), expansion = expand_scale(0, 0), +expand_limits_discrete <- function(limits, expansion = expand_scale(0, 0), coord_limits = c(NA, NA), range_continuous = NULL) { limit_info <- expand_limits_discrete_trans( limits, - coord_limits, expansion, + coord_limits, range_continuous = range_continuous ) limit_info$continuous_range } -expand_limits_continuous_trans <- function(limits, coord_limits = c(NA, NA), - expansion = expand_scale(0, 0), trans = identity_trans()) { +expand_limits_continuous_trans <- function(limits, expansion = expand_scale(0, 0), + coord_limits = c(NA, NA), trans = identity_trans()) { # let non-NA coord_limits override the scale limits limits <- ifelse(is.na(coord_limits), limits, coord_limits) @@ -183,8 +186,8 @@ expand_limits_continuous_trans <- function(limits, coord_limits = c(NA, NA), ) } -expand_limits_discrete_trans <- function(limits, coord_limits = c(NA, NA), - expansion = expand_scale(0, 0), trans = identity_trans(), +expand_limits_discrete_trans <- function(limits, expansion = expand_scale(0, 0), + coord_limits = c(NA, NA), trans = identity_trans(), range_continuous = NULL) { n_limits <- length(limits) @@ -193,18 +196,18 @@ expand_limits_discrete_trans <- function(limits, coord_limits = c(NA, NA), is_only_discrete <- is.null(range_continuous) if (is_empty) { - expand_limits_continuous_trans(c(0, 1), coord_limits, expansion, trans) + expand_limits_continuous_trans(c(0, 1), expansion, coord_limits, trans) } else if (is_only_continuous) { - expand_limits_continuous_trans(range_continuous, coord_limits, expansion, trans) + expand_limits_continuous_trans(range_continuous, expansion, coord_limits, trans) } else if (is_only_discrete) { - expand_limits_continuous_trans(c(1, n_limits), coord_limits, expansion, trans) + expand_limits_continuous_trans(c(1, n_limits), expansion, coord_limits, trans) } else { # continuous and discrete - limit_info_discrete <- expand_limits_continuous_trans(c(1, n_limits), coord_limits, expansion, trans) + limit_info_discrete <- expand_limits_continuous_trans(c(1, n_limits), expansion, coord_limits, trans) # don't expand continuous range if there is also a discrete range limit_info_continuous <- expand_limits_continuous_trans( - range_continuous, coord_limits, expand_scale(0, 0), trans + range_continuous, expand_scale(0, 0), coord_limits, trans ) # prefer expanded discrete range, but allow continuous range to further expand the range From 3f52a428f23019fee877e7779edd04833b093c8e Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Mon, 24 Jun 2019 15:25:33 -0400 Subject: [PATCH 14/26] mention removal of `xtrans` and `ytrans` arguments in `coord_trans()` --- NEWS.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index a8a6d8b91c..80202b6db1 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,7 +2,9 @@ * `coord_trans()` now draws second axes and accepts `xlim`, `ylim`, and `expand` arguments to bring it up to feature parity with - `coord_cartesian()` (@paleolimbot, #2990). + `coord_cartesian()`. The `xtrans` and `ytrans` arguments that were + deprecated in version 1.0.1 in favour of `x` and `y` + were removed (@paleolimbot, #2990). * `coord_trans()` now calculates breaks using the expanded range (previously these were calculated using the unexpanded range, From a4f5bb94d23b8f101bcc385fbddc064a3c1fa760 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Mon, 24 Jun 2019 15:32:49 -0400 Subject: [PATCH 15/26] explicitly noRd expand_range4() (not exported), fix PR review comments in scale-expansion.r --- R/scale-expansion.r | 10 +++++++--- man/expand_range4.Rd | 22 ---------------------- 2 files changed, 7 insertions(+), 25 deletions(-) delete mode 100644 man/expand_range4.Rd diff --git a/R/scale-expansion.r b/R/scale-expansion.r index ebb7d02a89..8cd43eaaff 100644 --- a/R/scale-expansion.r +++ b/R/scale-expansion.r @@ -36,8 +36,10 @@ #' scale_y_continuous(expand = expand_scale(mult = .05)) #' expand_scale = function(mult = 0, add = 0) { - stopifnot(is.numeric(mult) && is.numeric(add)) - stopifnot((length(mult) %in% 1:2) && (length(add) %in% 1:2)) + stopifnot( + is.numeric(mult), (length(mult) %in% 1:2), + is.numeric(add), (length(add) %in% 1:2) + ) mult <- rep(mult, length.out = 2) add <- rep(add, length.out = 2) @@ -54,6 +56,8 @@ expand_scale = function(mult = 0, add = 0) { #' #' @return The expanded `limits` #' +#' @noRd +#' expand_range4 <- function(limits, expand) { stopifnot( is.numeric(expand), @@ -100,7 +104,7 @@ expand_default <- function(scale, discrete = expand_scale(add = 0.6), #' #' These functions calculate the continuous range in coordinate space #' and in scale space. Usually these can be calculated from -#' eachother using the coordinate system transformation, except +#' each other using the coordinate system transformation, except #' when transforming and expanding the scale limits results in values outside #' the domain of the transformation (e.g., a lower limit of 0 with a square root #' transformation). diff --git a/man/expand_range4.Rd b/man/expand_range4.Rd deleted file mode 100644 index cfdb8a7d8d..0000000000 --- a/man/expand_range4.Rd +++ /dev/null @@ -1,22 +0,0 @@ -% Generated by roxygen2: do not edit by hand -% Please edit documentation in R/scale-expansion.r -\name{expand_range4} -\alias{expand_range4} -\title{Expand a numeric range} -\usage{ -expand_range4(limits, expand) -} -\arguments{ -\item{limits}{A numeric vector of length 2 giving the -range to expand.} - -\item{expand}{A numeric vector of length 2 (\code{c(add, mult)}) -or length 4 (\code{c(mult_left, add_left, mult_right, add_right)}), -as generated by \code{\link[=expand_scale]{expand_scale()}}.} -} -\value{ -The expanded \code{limits} -} -\description{ -Expand a numeric range -} From 3f06b3e9b509d0680d1dbc051387f3c4eef89276 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Mon, 24 Jun 2019 15:35:31 -0400 Subject: [PATCH 16/26] loudly deprecate limx and limy --- R/coord-transform.r | 4 ++-- man/coord_trans.Rd | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/R/coord-transform.r b/R/coord-transform.r index ee0a28d5dc..03cdd9cafa 100644 --- a/R/coord-transform.r +++ b/R/coord-transform.r @@ -10,7 +10,7 @@ #' #' @inheritParams coord_cartesian #' @param x,y Transformers for x and y axes or their names. -#' @param limx,limy These are deprecated - use `xlim` and `ylim` instead. +#' @param limx,limy **Deprecated**: use `xlim` and `ylim` instead. #' @export #' @examples #' \donttest{ @@ -75,7 +75,7 @@ #' plot + coord_trans(x = "sqrt") #' } coord_trans <- function(x = "identity", y = "identity", xlim = NULL, ylim = NULL, - limx = NULL, limy = NULL, clip = "on", expand = TRUE) { + limx = "DEPRECATED", limy = "DEPRECATED", clip = "on", expand = TRUE) { if (!missing(limx)) { gg_dep("3.2.0", "`limx` argument is deprecated; please use `xlim` instead.") xlim <- limx diff --git a/man/coord_trans.Rd b/man/coord_trans.Rd index 4f2acb70e5..d109704ae5 100644 --- a/man/coord_trans.Rd +++ b/man/coord_trans.Rd @@ -5,8 +5,8 @@ \title{Transformed Cartesian coordinate system} \usage{ coord_trans(x = "identity", y = "identity", xlim = NULL, - ylim = NULL, limx = NULL, limy = NULL, clip = "on", - expand = TRUE) + ylim = NULL, limx = "DEPRECATED", limy = "DEPRECATED", + clip = "on", expand = TRUE) } \arguments{ \item{x, y}{Transformers for x and y axes or their names.} @@ -15,7 +15,7 @@ coord_trans(x = "identity", y = "identity", xlim = NULL, \item{ylim}{Limits for the x and y axes.} -\item{limx, limy}{These are deprecated - use \code{xlim} and \code{ylim} instead.} +\item{limx, limy}{\strong{Deprecated}: use \code{xlim} and \code{ylim} instead.} \item{clip}{Should drawing be clipped to the extent of the plot panel? A setting of \code{"on"} (the default) means yes, and a setting of \code{"off"} From f72bd7b61797461f22e091731e59aa4460990a6d Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Mon, 24 Jun 2019 15:40:13 -0400 Subject: [PATCH 17/26] calculate expansion prior to calling expand_limits_scale() --- R/coord-sf.R | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/R/coord-sf.R b/R/coord-sf.R index 567311a182..087d35330e 100644 --- a/R/coord-sf.R +++ b/R/coord-sf.R @@ -127,16 +127,10 @@ CoordSf <- ggproto("CoordSf", CoordCartesian, setup_panel_params = function(self, scale_x, scale_y, params = list()) { # Bounding box of the data - x_range <- expand_limits_scale( - scale_x, - expand_default(scale_x, expand = self$expand), - coord_limits = self$limits$x - ) - y_range <- expand_limits_scale( - scale_y, - expand_default(scale_y, expand = self$expand), - coord_limits = self$limits$y - ) + expansion_x <- expand_default(scale_x, expand = self$expand) + x_range <- expand_limits_scale(scale_x, expansion_x, coord_limits = self$limits$x) + expansion_y <- expand_default(scale_y, expand = self$expand) + y_range <- expand_limits_scale(scale_y, expansion_y, coord_limits = self$limits$y) bbox <- c( x_range[1], y_range[1], x_range[2], y_range[2] From d60d0a1068f9cb360de42ddc8306e9ad8dcfec36 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Mon, 24 Jun 2019 15:43:44 -0400 Subject: [PATCH 18/26] clarify coord_limits param in expand_limits_scale() --- R/scale-expansion.r | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/R/scale-expansion.r b/R/scale-expansion.r index 8cd43eaaff..9a19bf227a 100644 --- a/R/scale-expansion.r +++ b/R/scale-expansion.r @@ -113,7 +113,9 @@ expand_default <- function(scale, discrete = expand_scale(add = 0.6), #' @param limits The initial scale limits, in scale-transformed space. #' @param coord_limits The user-provided limits in scale-transformed space, #' which may include one more more NA values, in which case those limits -#' will fall back to the `limits`. +#' will fall back to the `limits`. In `expand_limits_scale()`, `coord_limits` +#' are in user data space and can be `NULL` (unspecified), since the transformation +#' from user to mapped space is different for each scale. #' @param expansion An expansion generated by [expand_scale()] or [expand_default()]. #' @param trans The coordinate system transformation. #' From cb3161582189dca41e320fccb29e8a4eb3f3e88c Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Mon, 24 Jun 2019 16:40:07 -0400 Subject: [PATCH 19/26] fix assign operator for expand_scale --- R/scale-expansion.r | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/scale-expansion.r b/R/scale-expansion.r index 9a19bf227a..e9e8773e7b 100644 --- a/R/scale-expansion.r +++ b/R/scale-expansion.r @@ -35,7 +35,7 @@ #' scale_x_discrete(expand = expand_scale(add = .6)) + #' scale_y_continuous(expand = expand_scale(mult = .05)) #' -expand_scale = function(mult = 0, add = 0) { +expand_scale <- function(mult = 0, add = 0) { stopifnot( is.numeric(mult), (length(mult) %in% 1:2), is.numeric(add), (length(add) %in% 1:2) From 16f57c2d7839acc67102e65e28cda8b4801fde1d Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Tue, 25 Jun 2019 10:51:59 -0400 Subject: [PATCH 20/26] remove last reference to gg_dep (#3057) --- R/coord-transform.r | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/coord-transform.r b/R/coord-transform.r index 03cdd9cafa..2bcc757cac 100644 --- a/R/coord-transform.r +++ b/R/coord-transform.r @@ -77,11 +77,11 @@ coord_trans <- function(x = "identity", y = "identity", xlim = NULL, ylim = NULL, limx = "DEPRECATED", limy = "DEPRECATED", clip = "on", expand = TRUE) { if (!missing(limx)) { - gg_dep("3.2.0", "`limx` argument is deprecated; please use `xlim` instead.") + warning("`limx` argument is deprecated; please use `xlim` instead.", call. = FALSE) xlim <- limx } if (!missing(limy)) { - gg_dep("3.2.0", "`limy` argument is deprecated; please use `ylim` instead.") + warning("`limy` argument is deprecated; please use `ylim` instead.", call. = FALSE) ylim <- limy } From 9f92894a1bed1c6bab5da5b758556f294b48812f Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Mon, 1 Jul 2019 16:38:59 -0400 Subject: [PATCH 21/26] rename expand_default to default_expansion --- R/bin.R | 2 +- R/coord-cartesian-.r | 2 +- R/coord-map.r | 2 +- R/coord-polar.r | 4 ++-- R/coord-sf.R | 4 ++-- R/coord-transform.r | 2 +- R/scale-expansion.r | 6 +++--- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/R/bin.R b/R/bin.R index a5e7ae85bc..55d898c846 100644 --- a/R/bin.R +++ b/R/bin.R @@ -102,7 +102,7 @@ bin_breaks_bins <- function(x_range, bins = 30, center = NULL, if (bins < 1) { stop("Need at least one bin.", call. = FALSE) } else if (zero_range(x_range)) { - # 0.1 is the same width as the expansion `expand_default()` gives for 0-width data + # 0.1 is the same width as the expansion `default_expansion()` gives for 0-width data width <- 0.1 } else if (bins == 1) { width <- diff(x_range) diff --git a/R/coord-cartesian-.r b/R/coord-cartesian-.r index 2b7af37a4f..8222604039 100644 --- a/R/coord-cartesian-.r +++ b/R/coord-cartesian-.r @@ -137,7 +137,7 @@ CoordCartesian <- ggproto("CoordCartesian", Coord, ) view_scales_from_scale <- function(scale, coord_limits = NULL, expand = TRUE) { - expansion <- expand_default(scale, expand = expand) + expansion <- default_expansion(scale, expand = expand) limits <- scale$get_limits() continuous_range <- expand_limits_scale(scale, expansion, limits, coord_limits = coord_limits) aesthetic <- scale$aesthetics[1] diff --git a/R/coord-map.r b/R/coord-map.r index 093894b3c1..f6a56ac89f 100644 --- a/R/coord-map.r +++ b/R/coord-map.r @@ -181,7 +181,7 @@ CoordMap <- ggproto("CoordMap", Coord, for (n in c("x", "y")) { scale <- get(paste0("scale_", n)) limits <- self$limits[[n]] - range <- expand_limits_scale(scale, expand_default(scale), coord_limits = limits) + range <- expand_limits_scale(scale, default_expansion(scale), coord_limits = limits) ranges[[n]] <- range } diff --git a/R/coord-polar.r b/R/coord-polar.r index 5c78079957..fd5d44fa3f 100644 --- a/R/coord-polar.r +++ b/R/coord-polar.r @@ -112,9 +112,9 @@ CoordPolar <- ggproto("CoordPolar", Coord, limits <- self$limits[[n]] if (self$theta == n) { - expansion <- expand_default(scale, c(0, 0.5), c(0, 0)) + expansion <- default_expansion(scale, c(0, 0.5), c(0, 0)) } else { - expansion <- expand_default(scale, c(0, 0), c(0, 0)) + expansion <- default_expansion(scale, c(0, 0), c(0, 0)) } range <- expand_limits_scale(scale, expansion, coord_limits = limits) diff --git a/R/coord-sf.R b/R/coord-sf.R index 087d35330e..b73227c7e7 100644 --- a/R/coord-sf.R +++ b/R/coord-sf.R @@ -127,9 +127,9 @@ CoordSf <- ggproto("CoordSf", CoordCartesian, setup_panel_params = function(self, scale_x, scale_y, params = list()) { # Bounding box of the data - expansion_x <- expand_default(scale_x, expand = self$expand) + expansion_x <- default_expansion(scale_x, expand = self$expand) x_range <- expand_limits_scale(scale_x, expansion_x, coord_limits = self$limits$x) - expansion_y <- expand_default(scale_y, expand = self$expand) + expansion_y <- default_expansion(scale_y, expand = self$expand) y_range <- expand_limits_scale(scale_y, expansion_y, coord_limits = self$limits$y) bbox <- c( x_range[1], y_range[1], diff --git a/R/coord-transform.r b/R/coord-transform.r index 2bcc757cac..7b61c82094 100644 --- a/R/coord-transform.r +++ b/R/coord-transform.r @@ -178,7 +178,7 @@ transform_value <- function(trans, value, range) { } train_trans <- function(scale, coord_limits, trans, name, expand = TRUE) { - expansion <- expand_default(scale, expand = expand) + expansion <- default_expansion(scale, expand = expand) scale_trans <- scale$trans %||% identity_trans() coord_limits <- coord_limits %||% scale_trans$inverse(c(NA, NA)) diff --git a/R/scale-expansion.r b/R/scale-expansion.r index e9e8773e7b..0b65ebd98b 100644 --- a/R/scale-expansion.r +++ b/R/scale-expansion.r @@ -91,8 +91,8 @@ expand_range4 <- function(limits, expand) { #' @return One of `discrete`, `continuous`, or `scale$expand` #' @noRd #' -expand_default <- function(scale, discrete = expand_scale(add = 0.6), - continuous = expand_scale(mult = 0.05), expand = TRUE) { +default_expansion <- function(scale, discrete = expand_scale(add = 0.6), + continuous = expand_scale(mult = 0.05), expand = TRUE) { if (!expand) { return(expand_scale(0, 0)) } @@ -116,7 +116,7 @@ expand_default <- function(scale, discrete = expand_scale(add = 0.6), #' will fall back to the `limits`. In `expand_limits_scale()`, `coord_limits` #' are in user data space and can be `NULL` (unspecified), since the transformation #' from user to mapped space is different for each scale. -#' @param expansion An expansion generated by [expand_scale()] or [expand_default()]. +#' @param expansion An expansion generated by [expand_scale()] or [default_expansion()]. #' @param trans The coordinate system transformation. #' #' @return A list with components `continuous_range`, which is the From 73125a89c5a9fcccbd6c018c65743d59f03dfbbf Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Mon, 1 Jul 2019 17:08:40 -0400 Subject: [PATCH 22/26] rename `expand_scale()` to `expansion()` --- NAMESPACE | 1 + R/scale-.r | 12 +++--- R/scale-discrete-.r | 2 +- R/scale-expansion.r | 58 +++++++++++++++------------ _pkgdown.yml | 2 +- man/continuous_scale.Rd | 2 +- man/discrete_scale.Rd | 2 +- man/{expand_scale.Rd => expansion.Rd} | 13 +++--- man/ggplot2-ggproto.Rd | 2 +- man/scale_continuous.Rd | 2 +- man/scale_date.Rd | 2 +- man/scale_discrete.Rd | 4 +- man/scale_gradient.Rd | 2 +- man/scale_grey.Rd | 2 +- man/scale_hue.Rd | 2 +- man/scale_size.Rd | 2 +- tests/testthat/test-scale-expansion.r | 14 +++---- 17 files changed, 67 insertions(+), 57 deletions(-) rename man/{expand_scale.Rd => expansion.Rd} (84%) diff --git a/NAMESPACE b/NAMESPACE index 41405a260e..fe651a8765 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -290,6 +290,7 @@ export(ensym) export(ensyms) export(expand_limits) export(expand_scale) +export(expansion) export(expr) export(facet_grid) export(facet_null) diff --git a/R/scale-.r b/R/scale-.r index fd9b8be360..7e51852dc9 100644 --- a/R/scale-.r +++ b/R/scale-.r @@ -62,7 +62,7 @@ #' [guides()] for more information. #' @param 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 [expand_scale()] +#' away from the axes. Use the convenience function [expansion()] #' to generate the values for the `expand` argument. The defaults are to #' expand the scale by 5\% on each side for continuous variables, and by #' 0.6 units on each side for discrete variables. @@ -253,7 +253,7 @@ discrete_scale <- function(aesthetics, scale_name, palette, name = waiver(), #' - `dimension()` For continuous scales, the dimension is the same concept as the limits. #' For discrete scales, `dimension()` returns a continuous range, where the limits #' would be placed at integer positions. `dimension()` optionally expands -#' this range given an expantion of length 4 (see [expand_scale()]). +#' this range given an expantion of length 4 (see [expansion()]). #' #' - `break_info()` Returns a `list()` with calculated values needed for the `Coord` #' to transform values in transformed data space. Axis and grid guides also use @@ -380,7 +380,7 @@ Scale <- ggproto("Scale", NULL, } }, - dimension = function(self, expand = expand_scale(0, 0), limits = self$get_limits()) { + dimension = function(self, expand = expansion(0, 0), limits = self$get_limits()) { stop("Not implemented", call. = FALSE) }, @@ -487,7 +487,7 @@ ScaleContinuous <- ggproto("ScaleContinuous", Scale, self$rescaler(x, from = range) }, - dimension = function(self, expand = expand_scale(0, 0), limits = self$get_limits()) { + dimension = function(self, expand = expansion(0, 0), limits = self$get_limits()) { expand_limits_scale(self, expand, limits) }, @@ -688,8 +688,8 @@ ScaleDiscrete <- ggproto("ScaleDiscrete", Scale, rescale(x, match(as.character(x), limits), from = range) }, - dimension = function(self, expand = expand_scale(0, 0), limits = self$get_limits()) { - expand_limits_discrete(limits, expansion = expand) + dimension = function(self, expand = expansion(0, 0), limits = self$get_limits()) { + expand_limits_discrete(limits, expand = expand) }, get_breaks = function(self, limits = self$get_limits()) { diff --git a/R/scale-discrete-.r b/R/scale-discrete-.r index e2307c17d3..9dc3dc2537 100644 --- a/R/scale-discrete-.r +++ b/R/scale-discrete-.r @@ -105,7 +105,7 @@ ScaleDiscretePosition <- ggproto("ScaleDiscretePosition", ScaleDiscrete, rescale(self$map(x, limits = limits), from = range) }, - dimension = function(self, expand = expand_scale(0, 0), limits = self$get_limits()) { + dimension = function(self, expand = expansion(0, 0), limits = self$get_limits()) { expand_limits_scale(self, expand, limits) }, diff --git a/R/scale-expansion.r b/R/scale-expansion.r index 0b65ebd98b..de152bf18c 100644 --- a/R/scale-expansion.r +++ b/R/scale-expansion.r @@ -21,21 +21,21 @@ #' # No space below the bars but 10% above them #' ggplot(mtcars) + #' geom_bar(aes(x = factor(cyl))) + -#' scale_y_continuous(expand = expand_scale(mult = c(0, .1))) +#' scale_y_continuous(expand = expansion(mult = c(0, .1))) #' #' # Add 2 units of space on the left and right of the data #' ggplot(subset(diamonds, carat > 2), aes(cut, clarity)) + #' geom_jitter() + -#' scale_x_discrete(expand = expand_scale(add = 2)) +#' scale_x_discrete(expand = expansion(add = 2)) #' #' # Reproduce the default range expansion used #' # when the 'expand' argument is not specified #' ggplot(subset(diamonds, carat > 2), aes(cut, price)) + #' geom_jitter() + -#' scale_x_discrete(expand = expand_scale(add = .6)) + -#' scale_y_continuous(expand = expand_scale(mult = .05)) +#' scale_x_discrete(expand = expansion(add = .6)) + +#' scale_y_continuous(expand = expansion(mult = .05)) #' -expand_scale <- function(mult = 0, add = 0) { +expansion <- function(mult = 0, add = 0) { stopifnot( is.numeric(mult), (length(mult) %in% 1:2), is.numeric(add), (length(add) %in% 1:2) @@ -46,13 +46,19 @@ expand_scale <- function(mult = 0, add = 0) { c(mult[1], add[1], mult[2], add[2]) } +#' @rdname expansion +#' @export +expand_scale <- function(mult = 0, add = 0) { + expansion(mult, add) +} + #' Expand a numeric range #' #' @param limits A numeric vector of length 2 giving the #' range to expand. #' @param expand A numeric vector of length 2 (`c(add, mult)`) #' or length 4 (`c(mult_left, add_left, mult_right, add_right)`), -#' as generated by [expand_scale()]. +#' as generated by [expansion()]. #' #' @return The expanded `limits` #' @@ -91,10 +97,10 @@ expand_range4 <- function(limits, expand) { #' @return One of `discrete`, `continuous`, or `scale$expand` #' @noRd #' -default_expansion <- function(scale, discrete = expand_scale(add = 0.6), - continuous = expand_scale(mult = 0.05), expand = TRUE) { +default_expansion <- function(scale, discrete = expansion(add = 0.6), + continuous = expansion(mult = 0.05), expand = TRUE) { if (!expand) { - return(expand_scale(0, 0)) + return(expansion(0, 0)) } scale$expand %|W|% if (scale$is_discrete()) discrete else continuous @@ -116,7 +122,7 @@ default_expansion <- function(scale, discrete = expand_scale(add = 0.6), #' will fall back to the `limits`. In `expand_limits_scale()`, `coord_limits` #' are in user data space and can be `NULL` (unspecified), since the transformation #' from user to mapped space is different for each scale. -#' @param expansion An expansion generated by [expand_scale()] or [default_expansion()]. +#' @param expand An expansion generated by [expansion()] or [default_expansion()]. #' @param trans The coordinate system transformation. #' #' @return A list with components `continuous_range`, which is the @@ -125,7 +131,7 @@ default_expansion <- function(scale, discrete = expand_scale(add = 0.6), #' #' @noRd #' -expand_limits_scale <- function(scale, expansion = expand_scale(0, 0), limits = waiver(), +expand_limits_scale <- function(scale, expand = expansion(0, 0), limits = waiver(), coord_limits = NULL) { limits <- limits %|W|% scale$get_limits() @@ -133,7 +139,7 @@ expand_limits_scale <- function(scale, expansion = expand_scale(0, 0), limits = coord_limits <- coord_limits %||% c(NA_real_, NA_real_) expand_limits_discrete( limits, - expansion, + expand, coord_limits, range_continuous = scale$range_c$range ) @@ -142,19 +148,19 @@ expand_limits_scale <- function(scale, expansion = expand_scale(0, 0), limits = # scales, which refuse to transform objects of the incorrect type coord_limits <- coord_limits %||% scale$trans$inverse(c(NA_real_, NA_real_)) coord_limits_scale <- scale$trans$transform(coord_limits) - expand_limits_continuous(limits, expansion, coord_limits_scale) + expand_limits_continuous(limits, expand, coord_limits_scale) } } -expand_limits_continuous <- function(limits, expansion = expand_scale(0, 0), coord_limits = c(NA, NA)) { - expand_limits_continuous_trans(limits, expansion, coord_limits)$continuous_range +expand_limits_continuous <- function(limits, expand = expansion(0, 0), coord_limits = c(NA, NA)) { + expand_limits_continuous_trans(limits, expand, coord_limits)$continuous_range } -expand_limits_discrete <- function(limits, expansion = expand_scale(0, 0), coord_limits = c(NA, NA), +expand_limits_discrete <- function(limits, expand = expansion(0, 0), coord_limits = c(NA, NA), range_continuous = NULL) { limit_info <- expand_limits_discrete_trans( limits, - expansion, + expand, coord_limits, range_continuous = range_continuous ) @@ -162,7 +168,7 @@ expand_limits_discrete <- function(limits, expansion = expand_scale(0, 0), coord limit_info$continuous_range } -expand_limits_continuous_trans <- function(limits, expansion = expand_scale(0, 0), +expand_limits_continuous_trans <- function(limits, expand = expansion(0, 0), coord_limits = c(NA, NA), trans = identity_trans()) { # let non-NA coord_limits override the scale limits @@ -174,9 +180,9 @@ expand_limits_continuous_trans <- function(limits, expansion = expand_scale(0, 0 # range expansion expects values in increasing order, which may not be true # for reciprocal/reverse transformations if (all(is.finite(continuous_range_coord)) && diff(continuous_range_coord) < 0) { - continuous_range_coord <- rev(expand_range4(rev(continuous_range_coord), expansion)) + continuous_range_coord <- rev(expand_range4(rev(continuous_range_coord), expand)) } else { - continuous_range_coord <- expand_range4(continuous_range_coord, expansion) + continuous_range_coord <- expand_range4(continuous_range_coord, expand) } final_scale_limits <- trans$inverse(continuous_range_coord) @@ -192,7 +198,7 @@ expand_limits_continuous_trans <- function(limits, expansion = expand_scale(0, 0 ) } -expand_limits_discrete_trans <- function(limits, expansion = expand_scale(0, 0), +expand_limits_discrete_trans <- function(limits, expand = expansion(0, 0), coord_limits = c(NA, NA), trans = identity_trans(), range_continuous = NULL) { @@ -202,18 +208,18 @@ expand_limits_discrete_trans <- function(limits, expansion = expand_scale(0, 0), is_only_discrete <- is.null(range_continuous) if (is_empty) { - expand_limits_continuous_trans(c(0, 1), expansion, coord_limits, trans) + expand_limits_continuous_trans(c(0, 1), expand, coord_limits, trans) } else if (is_only_continuous) { - expand_limits_continuous_trans(range_continuous, expansion, coord_limits, trans) + expand_limits_continuous_trans(range_continuous, expand, coord_limits, trans) } else if (is_only_discrete) { - expand_limits_continuous_trans(c(1, n_limits), expansion, coord_limits, trans) + expand_limits_continuous_trans(c(1, n_limits), expand, coord_limits, trans) } else { # continuous and discrete - limit_info_discrete <- expand_limits_continuous_trans(c(1, n_limits), expansion, coord_limits, trans) + limit_info_discrete <- expand_limits_continuous_trans(c(1, n_limits), expand, coord_limits, trans) # don't expand continuous range if there is also a discrete range limit_info_continuous <- expand_limits_continuous_trans( - range_continuous, expand_scale(0, 0), coord_limits, trans + range_continuous, expansion(0, 0), coord_limits, trans ) # prefer expanded discrete range, but allow continuous range to further expand the range diff --git a/_pkgdown.yml b/_pkgdown.yml index cee17af014..39364ea714 100644 --- a/_pkgdown.yml +++ b/_pkgdown.yml @@ -98,7 +98,7 @@ reference: - labs - lims - expand_limits - - expand_scale + - expansion - starts_with("scale_") - title: "Guides: axes and legends" diff --git a/man/continuous_scale.Rd b/man/continuous_scale.Rd index 754fd16fe8..6fcf161e52 100644 --- a/man/continuous_scale.Rd +++ b/man/continuous_scale.Rd @@ -75,7 +75,7 @@ bounds values with \code{NA}.} \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[=expand_scale]{expand_scale()}} +away from the axes. Use the convenience function \code{\link[=expansion]{expansion()}} to generate the values for the \code{expand} argument. The defaults are to expand the scale by 5\% on each side for continuous variables, and by 0.6 units on each side for discrete variables.} diff --git a/man/discrete_scale.Rd b/man/discrete_scale.Rd index 3085e9d00d..0aabfe02b1 100644 --- a/man/discrete_scale.Rd +++ b/man/discrete_scale.Rd @@ -49,7 +49,7 @@ and their order.} \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[=expand_scale]{expand_scale()}} +away from the axes. Use the convenience function \code{\link[=expansion]{expansion()}} to generate the values for the \code{expand} argument. The defaults are to expand the scale by 5\% on each side for continuous variables, and by 0.6 units on each side for discrete variables.} diff --git a/man/expand_scale.Rd b/man/expansion.Rd similarity index 84% rename from man/expand_scale.Rd rename to man/expansion.Rd index 31bec3d23e..c32c04a53c 100644 --- a/man/expand_scale.Rd +++ b/man/expansion.Rd @@ -1,9 +1,12 @@ % Generated by roxygen2: do not edit by hand % Please edit documentation in R/scale-expansion.r -\name{expand_scale} +\name{expansion} +\alias{expansion} \alias{expand_scale} \title{Generate expansion vector for scales} \usage{ +expansion(mult = 0, add = 0) + expand_scale(mult = 0, add = 0) } \arguments{ @@ -28,18 +31,18 @@ add some space between the data and the axes. # No space below the bars but 10\% above them ggplot(mtcars) + geom_bar(aes(x = factor(cyl))) + - scale_y_continuous(expand = expand_scale(mult = c(0, .1))) + scale_y_continuous(expand = expansion(mult = c(0, .1))) # Add 2 units of space on the left and right of the data ggplot(subset(diamonds, carat > 2), aes(cut, clarity)) + geom_jitter() + - scale_x_discrete(expand = expand_scale(add = 2)) + scale_x_discrete(expand = expansion(add = 2)) # Reproduce the default range expansion used # when the 'expand' argument is not specified ggplot(subset(diamonds, carat > 2), aes(cut, price)) + geom_jitter() + - scale_x_discrete(expand = expand_scale(add = .6)) + - scale_y_continuous(expand = expand_scale(mult = .05)) + scale_x_discrete(expand = expansion(add = .6)) + + scale_y_continuous(expand = expansion(mult = .05)) } diff --git a/man/ggplot2-ggproto.Rd b/man/ggplot2-ggproto.Rd index 7c030bfb2e..4ea90864a6 100644 --- a/man/ggplot2-ggproto.Rd +++ b/man/ggplot2-ggproto.Rd @@ -460,7 +460,7 @@ These methods are only valid for position (x and y) scales: \item \code{dimension()} For continuous scales, the dimension is the same concept as the limits. For discrete scales, \code{dimension()} returns a continuous range, where the limits would be placed at integer positions. \code{dimension()} optionally expands -this range given an expantion of length 4 (see \code{\link[=expand_scale]{expand_scale()}}). +this range given an expantion of length 4 (see \code{\link[=expansion]{expansion()}}). \item \code{break_info()} Returns a \code{list()} with calculated values needed for the \code{Coord} to transform values in transformed data space. Axis and grid guides also use these values to draw guides. This is called with diff --git a/man/scale_continuous.Rd b/man/scale_continuous.Rd index 7baeff1789..ca74e2ad5e 100644 --- a/man/scale_continuous.Rd +++ b/man/scale_continuous.Rd @@ -79,7 +79,7 @@ new limits \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[=expand_scale]{expand_scale()}} +away from the axes. Use the convenience function \code{\link[=expansion]{expansion()}} to generate the values for the \code{expand} argument. The defaults are to expand the scale by 5\% on each side for continuous variables, and by 0.6 units on each side for discrete variables.} diff --git a/man/scale_date.Rd b/man/scale_date.Rd index 696312c1a6..a48f8b01d4 100644 --- a/man/scale_date.Rd +++ b/man/scale_date.Rd @@ -99,7 +99,7 @@ new limits \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[=expand_scale]{expand_scale()}} +away from the axes. Use the convenience function \code{\link[=expansion]{expansion()}} to generate the values for the \code{expand} argument. The defaults are to expand the scale by 5\% on each side for continuous variables, and by 0.6 units on each side for discrete variables.} diff --git a/man/scale_discrete.Rd b/man/scale_discrete.Rd index 9688612709..73697e91d2 100644 --- a/man/scale_discrete.Rd +++ b/man/scale_discrete.Rd @@ -52,7 +52,7 @@ 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[=expand_scale]{expand_scale()}} +away from the axes. Use the convenience function \code{\link[=expansion]{expansion()}} to generate the values for the \code{expand} argument. The defaults are to expand the scale by 5\% on each side for continuous variables, and by 0.6 units on each side for discrete variables.} @@ -65,7 +65,7 @@ expand the scale by 5\% on each side for continuous variables, and by \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[=expand_scale]{expand_scale()}} +away from the axes. Use the convenience function \code{\link[=expansion]{expansion()}} to generate the values for the \code{expand} argument. The defaults are to expand the scale by 5\% on each side for continuous variables, and by 0.6 units on each side for discrete variables.} diff --git a/man/scale_gradient.Rd b/man/scale_gradient.Rd index 6a741ca598..a69dcdf485 100644 --- a/man/scale_gradient.Rd +++ b/man/scale_gradient.Rd @@ -108,7 +108,7 @@ are defined in the scales package, and are called \code{_trans} (e.g., transformation with \code{\link[scales:trans_new]{scales::trans_new()}}.} \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[=expand_scale]{expand_scale()}} +away from the axes. Use the convenience function \code{\link[=expansion]{expansion()}} to generate the values for the \code{expand} argument. The defaults are to expand the scale by 5\% on each side for continuous variables, and by 0.6 units on each side for discrete variables.} diff --git a/man/scale_grey.Rd b/man/scale_grey.Rd index ee7ffd860f..0f4b1c491b 100644 --- a/man/scale_grey.Rd +++ b/man/scale_grey.Rd @@ -55,7 +55,7 @@ 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[=expand_scale]{expand_scale()}} +away from the axes. Use the convenience function \code{\link[=expansion]{expansion()}} to generate the values for the \code{expand} argument. The defaults are to expand the scale by 5\% on each side for continuous variables, and by 0.6 units on each side for discrete variables.} diff --git a/man/scale_hue.Rd b/man/scale_hue.Rd index 85d610e03a..e5c5d6ff36 100644 --- a/man/scale_hue.Rd +++ b/man/scale_hue.Rd @@ -59,7 +59,7 @@ 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[=expand_scale]{expand_scale()}} +away from the axes. Use the convenience function \code{\link[=expansion]{expansion()}} to generate the values for the \code{expand} argument. The defaults are to expand the scale by 5\% on each side for continuous variables, and by 0.6 units on each side for discrete variables.} diff --git a/man/scale_size.Rd b/man/scale_size.Rd index dd5a9918d3..43a284863e 100644 --- a/man/scale_size.Rd +++ b/man/scale_size.Rd @@ -133,7 +133,7 @@ transformation with \code{\link[scales:trans_new]{scales::trans_new()}}.} \code{\link[=guides]{guides()}} for more information.} \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[=expand_scale]{expand_scale()}} +away from the axes. Use the convenience function \code{\link[=expansion]{expansion()}} to generate the values for the \code{expand} argument. The defaults are to expand the scale by 5\% on each side for continuous variables, and by 0.6 units on each side for discrete variables.} diff --git a/tests/testthat/test-scale-expansion.r b/tests/testthat/test-scale-expansion.r index 4501a9065b..a5c0888bd3 100644 --- a/tests/testthat/test-scale-expansion.r +++ b/tests/testthat/test-scale-expansion.r @@ -8,12 +8,12 @@ test_that("expand_limits_continuous() can override limits", { }) test_that("expand_limits_continuous() expands limits", { - expect_identical(expand_limits_continuous(c(1, 2), expansion = expand_scale(add = 1)), c(0, 3)) + expect_identical(expand_limits_continuous(c(1, 2), expand = expansion(add = 1)), c(0, 3)) }) test_that("expand_limits_continuous() expands coord-supplied limits", { expect_identical( - expand_limits_continuous(c(1, 2), coord_limits = c(0, 4), expansion = expand_scale(add = 1)), + expand_limits_continuous(c(1, 2), coord_limits = c(0, 4), expand = expansion(add = 1)), c(-1, 5) ) }) @@ -21,25 +21,25 @@ test_that("expand_limits_continuous() expands coord-supplied limits", { test_that("expand_limits_continuous_trans() expands limits in coordinate space", { limit_info <- expand_limits_continuous_trans( c(1, 2), - expansion = expand_scale(add = 0.5), + expand = expansion(add = 0.5), trans = log10_trans() ) expect_identical( limit_info$continuous_range, - 10^(expand_range4(log10(c(1, 2)), expand_scale(add = 0.5))) + 10^(expand_range4(log10(c(1, 2)), expansion(add = 0.5))) ) expect_identical( limit_info$continuous_range_coord, - expand_range4(log10(c(1, 2)), expand_scale(add = 0.5)) + expand_range4(log10(c(1, 2)), expansion(add = 0.5)) ) }) test_that("introduced non-finite values fall back on scale limits", { limit_info <- expand_limits_continuous_trans( c(1, 100), - expansion = expand_scale(add = 2), + expand = expansion(add = 2), trans = sqrt_trans() ) @@ -92,7 +92,7 @@ test_that("expand_limits_discrete() can override limits with a both discrete and test_that("expand_limits_continuous_trans() works with inverted transformations", { limit_info <- expand_limits_continuous_trans( c(1, 2), - expansion = expand_scale(add = 1), + expand = expansion(add = 1), trans = reverse_trans() ) From 7acadc5c9365c728e710d6957e2b6a9007c3daa0 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Mon, 1 Jul 2019 17:10:52 -0400 Subject: [PATCH 23/26] add news bullet for expand_scale deprecation --- NEWS.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/NEWS.md b/NEWS.md index 80202b6db1..5d1fe22dbc 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,8 @@ # ggplot2 (development version) +* `expand_scale()` was soft-deprecated in favour of `expansion()` for setting + the `expand` argument of `x` and `y` scales (@paleolimbot). + * `coord_trans()` now draws second axes and accepts `xlim`, `ylim`, and `expand` arguments to bring it up to feature parity with `coord_cartesian()`. The `xtrans` and `ytrans` arguments that were From 2df6bdb41b73ca07a039afd9412dbada99af2cb1 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Mon, 1 Jul 2019 17:16:14 -0400 Subject: [PATCH 24/26] fix typo in expansion doc --- R/scale-expansion.r | 2 +- man/expansion.Rd | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/R/scale-expansion.r b/R/scale-expansion.r index de152bf18c..c571d2600b 100644 --- a/R/scale-expansion.r +++ b/R/scale-expansion.r @@ -3,7 +3,7 @@ #' #' This is a convenience function for generating scale expansion vectors #' for the `expand` argument of [scale_(x|y)_continuous][scale_x_continuous()] -#' and [scale_(x|y)_discrete][scale_x_discrete()]. The expansions vectors are used to +#' and [scale_(x|y)_discrete][scale_x_discrete()]. The expansion vectors are used to #' add some space between the data and the axes. #' #' @param mult vector of multiplicative range expansion factors. diff --git a/man/expansion.Rd b/man/expansion.Rd index c32c04a53c..cc6d6aa2fc 100644 --- a/man/expansion.Rd +++ b/man/expansion.Rd @@ -24,7 +24,7 @@ limit by \code{add[2]}.} \description{ This is a convenience function for generating scale expansion vectors for the \code{expand} argument of \link[=scale_x_continuous]{scale_(x|y)_continuous} -and \link[=scale_x_discrete]{scale_(x|y)_discrete}. The expansions vectors are used to +and \link[=scale_x_discrete]{scale_(x|y)_discrete}. The expansion vectors are used to add some space between the data and the axes. } \examples{ From 18220ccd640cd18e856479b83016a3774ff97453 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Mon, 1 Jul 2019 17:20:32 -0400 Subject: [PATCH 25/26] add deprecation warning for expand_scale() --- R/scale-expansion.r | 1 + tests/testthat/test-scale-expansion.r | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/R/scale-expansion.r b/R/scale-expansion.r index c571d2600b..ee0e6952b4 100644 --- a/R/scale-expansion.r +++ b/R/scale-expansion.r @@ -49,6 +49,7 @@ expansion <- function(mult = 0, add = 0) { #' @rdname expansion #' @export expand_scale <- function(mult = 0, add = 0) { + .Deprecated(msg = "`expand_scale()` is deprecated; use `expansion()` instead.") expansion(mult, add) } diff --git a/tests/testthat/test-scale-expansion.r b/tests/testthat/test-scale-expansion.r index a5c0888bd3..f4f0e829b5 100644 --- a/tests/testthat/test-scale-expansion.r +++ b/tests/testthat/test-scale-expansion.r @@ -1,4 +1,8 @@ +test_that("expand_scale() produces a deprecation warning", { + expect_warning(expand_scale(), "deprecated") +}) + # Expanding continuous scales ----------------------------------------- test_that("expand_limits_continuous() can override limits", { From 7773fc62c3fb1e6923402d82a558580e13d14072 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Mon, 1 Jul 2019 17:25:12 -0400 Subject: [PATCH 26/26] fix dep notice --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 5d1fe22dbc..7d61546d9a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,6 @@ # ggplot2 (development version) -* `expand_scale()` was soft-deprecated in favour of `expansion()` for setting +* `expand_scale()` was deprecated in favour of `expansion()` for setting the `expand` argument of `x` and `y` scales (@paleolimbot). * `coord_trans()` now draws second axes and accepts `xlim`, `ylim`,