Fix labels/breaks/limits interactions in bin guides (#4849)

NEWS.md

@@ -1,5 +1,8 @@
 # ggplot2 (development version)
 
+* Fix various issues with how `labels`, `breaks`, `limits`, and `show.limits`
+  interact in the different binning guides (@thomasp85, #4831)
+
 * Automatic break calculation now squishes the scale limits to the domain
   of the transformation. This allows `scale_{x/y}_sqrt()` to find breaks at 0   
   when appropriate (@teunbrand, #980).

R/guide-bins.R

@@ -14,7 +14,10 @@
 #' @param axis.arrow A call to `arrow()` to specify arrows at the end of the
 #'   axis line, thus showing an open interval.
 #' @param show.limits Logical. Should the limits of the scale be shown with
-#'   labels and ticks.
+#'   labels and ticks. Default is `NULL` meaning it will take the value from the
+#'   scale. This argument is ignored if `labels` is given as a vector of
+#'   values. If one or both of the limits is also given in `breaks` it will be
+#'   shown irrespective of the value of `show.limits`.
 #'
 #' @section Use with discrete scale:
 #' This guide is intended to show binned data and work together with ggplot2's
@@ -137,15 +140,25 @@ guide_train.bins <- function(guide, scale, aesthetic = NULL) {
   if (length(breaks) == 0 || all(is.na(breaks))) {
     return()
   }
+  show_limits <- guide$show.limits %||% scale$show.limits %||% FALSE
+  if (show_limits && (is.character(scale$labels) || is.numeric(scale$labels))) {
+    cli::cli_warn(c(
+      "{.arg show.limits} is ignored when {.arg labels} are given as a character vector",
+      "i" = "Either add the limits to {.arg breaks} or provide a function for {.arg labels}"
+    ))
+    show_limits <- FALSE
+  }
   # in the key data frame, use either the aesthetic provided as
   # argument to this function or, as a fall back, the first in the vector
   # of possible aesthetics handled by the scale
   aes_column_name <- aesthetic %||% scale$aesthetics[1]
 
   if (is.numeric(breaks)) {
     limits <- scale$get_limits()
-    breaks <- breaks[!breaks %in% limits]
-    all_breaks <- c(limits[1], breaks, limits[2])
+    if (!is.numeric(scale$breaks)) {
+      breaks <- breaks[!breaks %in% limits]
+    }
+    all_breaks <- unique(c(limits[1], breaks, limits[2]))
     bin_at <- all_breaks[-1] - diff(all_breaks) / 2
   } else {
     # If the breaks are not numeric it is used with a discrete scale. We check
@@ -162,10 +175,29 @@ guide_train.bins <- function(guide, scale, aesthetic = NULL) {
       ))
     }
     all_breaks <- breaks[c(1, seq_along(bin_at) * 2)]
+    limits <- all_breaks[c(1, length(all_breaks))]
+    breaks <- all_breaks[-c(1, length(all_breaks))]
   }
   key <- new_data_frame(setNames(list(c(scale$map(bin_at), NA)), aes_column_name))
-  key$.label <- scale$get_labels(all_breaks)
-  guide$show.limits <- guide$show.limits %||% scale$show_limits %||% FALSE
+  labels <- scale$get_labels(breaks)
+  show_limits <- rep(show_limits, 2)
+  if (is.character(scale$labels) || is.numeric(scale$labels)) {
+    limit_lab <- c(NA, NA)
+  } else {
+    limit_lab <- scale$get_labels(limits)
+  }
+  if (!breaks[1] %in% limits) {
+    labels <- c(limit_lab[1], labels)
+  } else {
+    show_limits[1] <- TRUE
+  }
+  if (!breaks[length(breaks)] %in% limits) {
+    labels <- c(labels, limit_lab[2])
+  } else {
+    show_limits[2] <- TRUE
+  }
+  key$.label <- labels
+  guide$show.limits <- show_limits
 
   if (guide$reverse) {
     key <- key[rev(seq_len(nrow(key))), ]
@@ -245,9 +277,7 @@ guide_geom.bins <- function(guide, layers, default_mapping) {
 
 #' @export
 guide_gengrob.bins <- function(guide, theme) {
-  if (!guide$show.limits) {
-    guide$key$.label[c(1, nrow(guide$key))] <- NA
-  }
+  guide$key$.label[c(1, nrow(guide$key))[!guide$show.limits]] <- NA
 
   # default setting
   if (guide$direction == "horizontal") {
@@ -332,9 +362,7 @@ guide_gengrob.bins <- function(guide, theme) {
       )
       ggname("guide.label", g)
     })
-    if (!guide$show.limits) {
-      grob.labels[c(1, length(grob.labels))] <- list(zeroGrob())
-    }
+    grob.labels[c(1, length(grob.labels))[!guide$show.limits]] <- list(zeroGrob())
   }
 
   label_widths <- width_cm(grob.labels)
@@ -514,9 +542,8 @@ guide_gengrob.bins <- function(guide, theme) {
     )
   }
   grob.ticks <- rep_len(list(grob.ticks), length(grob.labels))
-  if (!guide$show.limits) {
-    grob.ticks[c(1, length(grob.ticks))] <- list(zeroGrob())
-  }
+  grob.ticks[c(1, length(grob.ticks))[!guide$show.limits]] <- list(zeroGrob())
+
   # Create the gtable for the legend
   gt <- gtable(widths = unit(widths, "cm"), heights = unit(heights, "cm"))
   gt <- gtable_add_grob(

R/guide-colorsteps.R

@@ -6,8 +6,11 @@
 #'
 #' @param even.steps Should the rendered size of the bins be equal, or should
 #'   they be proportional to their length in the data space? Defaults to `TRUE`
-#' @param show.limits Should labels for the outer limits of the bins be printed?
-#'   Default is `NULL` which makes the guide use the setting from the scale
+#' @param show.limits Logical. Should the limits of the scale be shown with
+#'   labels and ticks. Default is `NULL` meaning it will take the value from the
+#'   scale. This argument is ignored if `labels` is given as a vector of
+#'   values. If one or both of the limits is also given in `breaks` it will be
+#'   shown irrespective of the value of `show.limits`.
 #' @param ticks A logical specifying if tick marks on the colourbar should be
 #'   visible.
 #' @inheritDotParams guide_colourbar -nbin -raster -ticks -available_aes
@@ -58,14 +61,24 @@ guide_colorsteps <- guide_coloursteps
 guide_train.colorsteps <- function(guide, scale, aesthetic = NULL) {
   breaks <- scale$get_breaks()
   breaks <- breaks[!is.na(breaks)]
+  show_limits <- guide$show.limits %||% scale$show.limits %||% FALSE
+  if (show_limits && (is.character(scale$labels) || is.numeric(scale$labels))) {
+    cli::cli_warn(c(
+      "{.arg show.limits} is ignored when {.arg labels} are given as a character vector",
+      "i" = "Either add the limits to {.arg breaks} or provide a function for {.arg labels}"
+    ))
+    show_limits <- FALSE
+  }
   if (guide$even.steps || !is.numeric(breaks)) {
     if (length(breaks) == 0 || all(is.na(breaks))) {
       return()
     }
     if (is.numeric(breaks)) {
       limits <- scale$get_limits()
-      breaks <- breaks[!breaks %in% limits]
-      all_breaks <- c(limits[1], breaks, limits[2])
+      if (!is.numeric(scale$breaks)) {
+        breaks <- breaks[!breaks %in% limits]
+      }
+      all_breaks <- unique(c(limits[1], breaks, limits[2]))
       bin_at <- all_breaks[-1] - diff(all_breaks) / 2
     } else {
       # If the breaks are not numeric it is used with a discrete scale. We check
@@ -91,7 +104,16 @@ guide_train.colorsteps <- function(guide, scale, aesthetic = NULL) {
     ticks <- new_data_frame(setNames(list(scale$map(breaks)), aesthetic %||% scale$aesthetics[1]))
     ticks$.value <- seq_along(breaks) - 0.5
     ticks$.label <- scale$get_labels(breaks)
-    guide$nbin <- length(breaks) + 1
+    guide$nbin <- length(breaks) + 1L
+    if (breaks[1] %in% limits) {
+      ticks$.value <- ticks$.value - 1L
+      ticks[[1]][1] <- NA
+      guide$nbin <- guide$nbin - 1L
+    }
+    if (breaks[length(breaks)] %in% limits) {
+      ticks[[1]][nrow(ticks)] <- NA
+      guide$nbin <- guide$nbin - 1L
+    }
     guide$key <- ticks
     guide$bar <- new_data_frame(list(colour = scale$map(bin_at), value = seq_along(bin_at) - 1), n = length(bin_at))
 
@@ -104,12 +126,18 @@ guide_train.colorsteps <- function(guide, scale, aesthetic = NULL) {
     guide <- NextMethod()
     limits <- scale$get_limits()
   }
-  if (guide$show.limits %||% scale$show.limits %||% FALSE) {
+  if (show_limits) {
     edges <- rescale(c(0, 1), to = guide$bar$value[c(1, nrow(guide$bar))], from = c(0.5, guide$nbin - 0.5) / guide$nbin)
     if (guide$reverse) edges <- rev(edges)
     guide$key <- guide$key[c(NA, seq_len(nrow(guide$key)), NA), , drop = FALSE]
     guide$key$.value[c(1, nrow(guide$key))] <- edges
     guide$key$.label[c(1, nrow(guide$key))] <- scale$get_labels(limits)
+    if (guide$key$.value[1] == guide$key$.value[2]) {
+      guide$key <- guide$key[-1,]
+    }
+    if (guide$key$.value[nrow(guide$key)-1] == guide$key$.value[nrow(guide$key)]) {
+      guide$key <- guide$key[-nrow(guide$key),]
+    }
   }
   guide
 }

tests/testthat/_snaps/guides.md

@@ -1,3 +1,13 @@
+# binning scales understand the different combinations of limits, breaks, labels, and show.limits
+
+    `show.limits` is ignored when `labels` are given as a character vector
+    i Either add the limits to `breaks` or provide a function for `labels`
+
+---
+
+    `show.limits` is ignored when `labels` are given as a character vector
+    i Either add the limits to `breaks` or provide a function for `labels`
+
 # axis_label_element_overrides errors when angles are outside the range [0, 90]
 
     Unrecognized `axis_position`: "test"