Consolidate coordinate scale expansion and limiting code

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

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 **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 = "DEPRECATED", limy = "DEPRECATED", clip = "on", expand = TRUE) {
+  if (!missing(limx)) {
+    warning("`limx` argument is deprecated; please use `xlim` instead.", call. = FALSE)
+    xlim <- limx
   }
-  if (!missing(ytrans)) {
-    gg_dep("1.0.1", "`ytrans` arguments is deprecated; please use `y` instead.")
-    y <- ytrans
+  if (!missing(limy)) {
+    warning("`limy` argument is deprecated; please use `ylim` instead.", call. = FALSE)
+    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,39 +177,51 @@ 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 <- default_expansion(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(),
+      expansion,
+      coord_limits,
+      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(),
+      expansion,
+      coord_limits,
+      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
+  # needs to be in increasing order for transform_value() to work
+  out$range <- 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)
+  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