Skip to content

Commit c9fbb28

Browse files
authored
Make ScaleDiscrete$map() compatible with vctrs-palettes (#6118)
* sprinkle vctrs over `ScaleDiscrete$map()` * add test * prevent guide from clogging up * add news bullet
1 parent a4264bc commit c9fbb28

File tree

3 files changed

+49
-10
lines changed

3 files changed

+49
-10
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+
* More stability for vctrs-based palettes (@teunbrand, #6117).
34
* Fixed regression in `guide_bins(reverse = TRUE)` (@teunbrand, #6183).
45
* New function family for setting parts of a theme. For example, you can now use
56
`theme_sub_axis(line, text, ticks, ticks.length, line)` as a substitute for

R/scale-.R

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -963,10 +963,10 @@ ScaleDiscrete <- ggproto("ScaleDiscrete", Scale,
963963
transform = identity,
964964

965965
map = function(self, x, limits = self$get_limits()) {
966-
limits <- limits[!is.na(limits)]
967-
n <- length(limits)
966+
limits <- vec_slice(limits, !is.na(limits))
967+
n <- vec_size(limits)
968968
if (n < 1) {
969-
return(rep(self$na.value, length(x)))
969+
return(vec_rep(self$na.value, vec_size(x)))
970970
}
971971
if (!is.null(self$n.breaks.cache) && self$n.breaks.cache == n) {
972972
pal <- self$palette.cache
@@ -982,21 +982,30 @@ ScaleDiscrete <- ggproto("ScaleDiscrete", Scale,
982982
self$n.breaks.cache <- n
983983
}
984984

985-
na_value <- if (self$na.translate) self$na.value else NA
986-
pal_names <- names(pal)
985+
na_value <- NA
986+
if (self$na.translate) {
987+
na_value <- self$na.value
988+
if (obj_is_list(pal) && !obj_is_list(na_value)) {
989+
# We prevent a casting error that occurs when mapping grid patterns
990+
na_value <- list(na_value)
991+
}
992+
}
993+
994+
pal_names <- vec_names(pal)
987995

988996
if (!is_null(pal_names)) {
989997
# if pal is named, limit the pal by the names first,
990998
# then limit the values by the pal
991-
pal[is.na(match(pal_names, limits))] <- na_value
992-
pal <- unname(pal)
999+
vec_slice(pal, is.na(match(pal_names, limits))) <- na_value
1000+
pal <- vec_set_names(pal, NULL)
9931001
limits <- pal_names
9941002
}
995-
pal <- c(pal, na_value)
996-
pal_match <- pal[match(as.character(x), limits, nomatch = length(pal))]
1003+
pal <- vec_c(pal, na_value)
1004+
pal_match <-
1005+
vec_slice(pal, match(as.character(x), limits, nomatch = vec_size(pal)))
9971006

9981007
if (!is.na(na_value)) {
999-
pal_match[is.na(x)] <- na_value
1008+
vec_slice(pal_match, is.na(x)) <- na_value
10001009
}
10011010
pal_match
10021011
},

tests/testthat/test-scales.R

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -747,3 +747,32 @@ test_that("discrete scales work with NAs in arbitrary positions", {
747747
expect_equal(test, output)
748748

749749
})
750+
751+
test_that("discrete scales can map to 2D structures", {
752+
753+
p <- ggplot(mtcars, aes(disp, mpg, colour = factor(cyl))) +
754+
geom_point()
755+
756+
# Test it can map to a vctrs rcrd class
757+
rcrd <- new_rcrd(list(a = LETTERS[1:3], b = 3:1))
758+
759+
ld <- layer_data(p + scale_colour_manual(values = rcrd, na.value = NA))
760+
expect_s3_class(ld$colour, "vctrs_rcrd")
761+
expect_length(ld$colour, nrow(mtcars))
762+
763+
# Test it can map to data.frames
764+
df <- data_frame0(a = LETTERS[1:3], b = 3:1)
765+
my_pal <- function(n) vec_slice(df, seq_len(n))
766+
767+
ld <- layer_data(p + discrete_scale("colour", palette = my_pal))
768+
expect_s3_class(ld$colour, "data.frame")
769+
expect_equal(dim(ld$colour), c(nrow(mtcars), ncol(df)))
770+
771+
# Test it can map to matrices
772+
mtx <- cbind(a = LETTERS[1:3], b = LETTERS[4:6])
773+
my_pal <- function(n) vec_slice(mtx, seq_len(n))
774+
775+
ld <- layer_data(p + discrete_scale("colour", palette = my_pal))
776+
expect_true(is.matrix(ld$colour))
777+
expect_equal(dim(ld$colour), c(nrow(mtcars), ncol(df)))
778+
})

0 commit comments

Comments
 (0)