Consolidate coordinate scale expansion and limiting code (#3380)

* move expantion-related functions to scale-expansion.r

* allow NA for xlim and/or ylim in coord functions

* get second axes to work with coord_trans()

* ensure that the reverse/reciprocal transformations work with coord_trans()

* mention removal of `xtrans` and `ytrans` arguments in `coord_trans()`

* rename `expand_scale()` to `expansion()`

Author: paleolimbot

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-expansion.r'
     'scale-gradient.r'
     'scale-grey.r'
     'scale-hue.r'

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)

NEWS.md

@@ -1,5 +1,28 @@
 # ggplot2 (development version)
 
+* `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`,
+  and `expand` arguments to bring it up to feature parity with 
+  `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, 
+  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`)

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

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)

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) {

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 <- default_expansion(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 <- expand_limits_scale(scale, expansion, limits, coord_limits = coord_limits)
   aesthetic <- scale$aesthetics[1]
 
   view_scales <- list(

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 <- expand_limits_scale(scale, default_expansion(scale), coord_limits = limits)
       ranges[[n]] <- range
     }
 

R/coord-polar.r

@@ -111,25 +111,21 @@ 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) {
+        expansion <- default_expansion(scale, c(0, 0.5), c(0, 0))
       } else {
-        range <- range(scale_transform(scale, limits))
+        expansion <- default_expansion(scale, c(0, 0),   c(0, 0))
       }
+      range <- expand_limits_scale(scale, expansion, coord_limits = limits)
 
       out <- scale$break_info(range)
       ret[[n]]$range <- out$range
       ret[[n]]$major <- out$major_source
       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
     }
 

R/coord-sf.R

@@ -127,8 +127,10 @@ 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)
+    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 <- 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],
       x_range[2], y_range[2]
@@ -466,14 +468,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)
-  }
-}