Skip to content

Commit f52de1c

Browse files
authored
Discard out-of-bounds binned breaks after generating labels (#6061)
* scales don't discard oob breaks * guides deal with NA-breaks better * `parse_binned_breaks()` preserves break length * add test * add news bullet * choose non-default scale for test
1 parent 1bb9230 commit f52de1c

File tree

5 files changed

+28
-8
lines changed

5 files changed

+28
-8
lines changed

NEWS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# ggplot2 (development version)
22

3+
* Fixed bug in out-of-bounds binned breaks (@teunbrand, #6054)
34
* Binned guides now accept expressions as labels (@teunbrand, #6005)
45
* (internal) `Scale$get_labels()` format expressions as lists.
56
* In non-orthogonal coordinate systems (`coord_sf()`, `coord_polar()` and

R/guide-bins.R

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,9 @@ GuideBins <- ggproto(
161161
key$.show <- NA
162162

163163
labels <- scale$get_labels(breaks)
164+
labels <- labels[!is.na(breaks)]
165+
breaks <- breaks[!is.na(breaks)]
166+
164167
if (is.character(scale$labels) || is.numeric(scale$labels) || is.expression(scale$labels)) {
165168
limit_lab <- c(NA, NA)
166169
} else {
@@ -335,19 +338,22 @@ GuideBins <- ggproto(
335338

336339
parse_binned_breaks <- function(scale, breaks = scale$get_breaks()) {
337340

338-
breaks <- breaks[!is.na(breaks)]
341+
if (is.waiver(scale$labels) || is.function(scale$labels)) {
342+
breaks <- breaks[!is.na(breaks)]
343+
}
339344
if (length(breaks) == 0) {
340345
return(NULL)
341346
}
342347

343348
if (is.numeric(breaks)) {
344-
breaks <- sort(breaks)
345349
limits <- scale$get_limits()
346350
if (!is.numeric(scale$breaks)) {
347-
breaks <- breaks[!breaks %in% limits]
351+
breaks[breaks %in% limits] <- NA
348352
}
349-
breaks <- oob_discard(breaks, limits)
353+
breaks <- oob_censor(breaks, limits)
350354
all_breaks <- unique0(c(limits[1], breaks, limits[2]))
355+
# Sorting drops NAs on purpose here
356+
all_breaks <- sort(all_breaks, na.last = NA)
351357
bin_at <- all_breaks[-1] - diff(all_breaks) / 2
352358
} else {
353359
bin_at <- breaks

R/guide-colorsteps.R

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,11 +112,13 @@ GuideColoursteps <- ggproto(
112112

113113
key <- data_frame0(!!aesthetic := scale$map(breaks))
114114
if (even.steps) {
115-
key$.value <- seq_along(breaks)
115+
key$.value <- NA_integer_
116+
key$.value[!is.na(breaks)] <- seq_along(breaks[!is.na(breaks)])
116117
} else {
117118
key$.value <- breaks
118119
}
119120
key$.label <- scale$get_labels(breaks)
121+
key <- vec_slice(key, !is.na(breaks))
120122

121123
if (breaks[1] %in% limits) {
122124
key$.value <- key$.value - 1L

R/scale-.R

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1307,9 +1307,6 @@ ScaleBinned <- ggproto("ScaleBinned", Scale,
13071307
breaks <- self$breaks
13081308
}
13091309

1310-
# Breaks must be within limits
1311-
breaks <- oob_discard(breaks, sort(limits))
1312-
13131310
self$breaks <- breaks
13141311

13151312
transformation$transform(breaks)

tests/testthat/test-guides.R

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,20 @@ test_that("bins can be parsed by guides for all scale types", {
190190
)
191191
})
192192

193+
test_that("binned breaks can have hardcoded labels when oob", {
194+
195+
sc <- scale_colour_steps(breaks = 1:3, labels = as.character(1:3))
196+
sc$train(c(1, 2))
197+
198+
g <- guide_bins()
199+
key <- g$train(scale = sc, aesthetic = "colour")$key
200+
expect_equal(key$.label, c("1", "2"))
201+
202+
g <- guide_coloursteps()
203+
key <- g$train(scale = sc, aesthetic = "colour")$key
204+
expect_equal(key$.label, c("1", "2"))
205+
})
206+
193207
# Visual tests ------------------------------------------------------------
194208

195209
test_that("guides are positioned correctly", {

0 commit comments

Comments
 (0)