Merge pull request #5104 from teunbrand/binned_limits

Iron out binned limits and breaks

Author: teunbrand

NEWS.md

@@ -1,5 +1,7 @@
 # ggplot2 (development version)
 
+* Automatic breaks in `scale_*_binned()` should no longer be out-of-bounds,
+  and automatic limits expand to include these (@teunbrand, #5095, #5100).
 * Renamed computed aesthetic in `stat_ecdf()` to `ecdf`, to prevent incorrect
   scale transformations (@teunbrand, #5113 and #5112).
 * Fixed misbehaviour of `draw_key_boxplot()` and `draw_key_crossbar()` with 

R/scale-.r

@@ -1077,6 +1077,7 @@ ScaleBinned <- ggproto("ScaleBinned", Scale,
         breaks <- seq(limits[1], limits[2], length.out = n.breaks + 2)
         breaks <- breaks[-c(1, length(breaks))]
       }
+      breaks <- oob_discard(breaks, limits)
 
       # Ensure terminal bins are same width if limits not set
       if (is.null(self$limits)) {

R/scale-binned.R

@@ -70,6 +70,7 @@ ScaleBinnedPosition <- ggproto("ScaleBinnedPosition", ScaleBinned,
 
   map = function(self, x, limits = self$get_limits()) {
     breaks <- self$get_breaks(limits)
+    limits <- self$get_limits() # get_breaks() may have updated this
     all_breaks <- unique0(sort(c(limits[1], breaks, limits[2])))
 
     if (self$after.stat) {

tests/testthat/test-scale-binned.R

@@ -5,6 +5,45 @@ test_that("binned scales only support continuous data", {
   expect_snapshot_error(ggplot_build(p))
 })
 
+test_that("binned scales limits can expand to fit breaks", {
+  # See also #5095
+
+  scale <- scale_x_binned(right = FALSE, show.limits = TRUE)
+  scale$train(c(14, 29))
+
+  limits <- scale$get_limits()
+  breaks <- scale$get_breaks()
+  new_limits <- scale$get_limits()
+
+  # Positive control
+  expect_equal(limits,     c(14, 29))
+  # Test case, should have been updated in break calculation
+  expect_equal(new_limits, c(14, 30))
+
+  # Negative control
+  # Now, new limits should not be updated because limits were given instead
+  # of computed
+  scale <- scale_x_binned(right = FALSE, show.limits = TRUE,
+                          limits = c(14, 29))
+  limits <- scale$get_limits()
+  breaks <- scale$get_breaks()
+  new_limits <- scale$get_limits()
+
+  expect_equal(limits, new_limits)
+})
+
+test_that("binned limits should not compute out-of-bounds breaks", {
+  scale <- scale_x_binned(n.breaks = 10)
+  scale$train(c(1, 9))
+
+  limits <- scale$get_limits()
+  breaks <- scale$get_breaks()
+  expect_length(breaks, 7) # Not the requested 10 due to oob discarding
+  expect_true(all(
+    breaks > limits[1] & breaks < limits[2]
+  ))
+})
+
 test_that('binned scales can calculate breaks on dates', {
 
   data <- seq(as.Date("2000-01-01"), as.Date("2020-01-01"), length.out = 100)