Skip to content

Commit c679d43

Browse files
authored
Preserve NAs in discrete palettes (#5949)
* choose `na.value` when unmatched * add test * add news bullet
1 parent f5782ff commit c679d43

File tree

3 files changed

+38
-12
lines changed

3 files changed

+38
-12
lines changed

NEWS.md

+2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# ggplot2 (development version)
22

3+
* Missing values from discrete palettes are no longer translated
4+
(@teunbrand, #5929).
35
* Fixed bug in `facet_grid(margins = TRUE)` when using expresssions
46
(@teunbrand, #1864).
57
* `geom_step()` now supports the `orientation` argument (@teunbrand, #5936).

R/scale-.R

+12-12
Original file line numberDiff line numberDiff line change
@@ -973,23 +973,23 @@ ScaleDiscrete <- ggproto("ScaleDiscrete", Scale,
973973
self$n.breaks.cache <- n
974974
}
975975

976-
if (!is_null(names(pal))) {
976+
na_value <- if (self$na.translate) self$na.value else NA
977+
pal_names <- names(pal)
978+
979+
if (!is_null(pal_names)) {
977980
# if pal is named, limit the pal by the names first,
978981
# then limit the values by the pal
979-
idx_nomatch <- is.na(match(names(pal), limits))
980-
pal[idx_nomatch] <- NA
981-
pal_match <- pal[match(as.character(x), names(pal))]
982-
pal_match <- unname(pal_match)
983-
} else {
984-
# if pal is not named, limit the values directly
985-
pal_match <- pal[match(as.character(x), limits)]
982+
pal[is.na(match(pal_names, limits))] <- na_value
983+
pal <- unname(pal)
984+
limits <- pal_names
986985
}
986+
pal <- c(pal, na_value)
987+
pal_match <- pal[match(as.character(x), limits, nomatch = length(pal))]
987988

988-
if (self$na.translate) {
989-
ifelse(is.na(x) | is.na(pal_match), self$na.value, pal_match)
990-
} else {
991-
pal_match
989+
if (!is.na(na_value)) {
990+
pal_match[is.na(x)] <- na_value
992991
}
992+
pal_match
993993
},
994994

995995
rescale = function(self, x, limits = self$get_limits(), range = c(1, length(limits))) {

tests/testthat/test-scale-manual.R

+24
Original file line numberDiff line numberDiff line change
@@ -152,3 +152,27 @@ test_that("limits and breaks (#4619)", {
152152
expect_equal(s3$map(c("4", "6", "8")), c("a", "b", "c"))
153153
expect_equal(s3$break_positions(), c("a", "c"))
154154
})
155+
156+
test_that("NAs from palette are not translated (#5929)", {
157+
158+
s1 <- scale_colour_manual(
159+
values = c("4" = "a", "6" = NA, "8" = "c"),
160+
na.translate = TRUE, na.value = "x"
161+
)
162+
s1$train(c("8", "6", "4"))
163+
expect_equal(s1$map(c("4", "6", "8", "10")), c("a", NA, "c", "x"))
164+
165+
s2 <- scale_colour_manual(
166+
values = c("4" = "a", "6" = NA, "8" = "c"),
167+
na.translate = TRUE, na.value = NA
168+
)
169+
s2$train(c("8", "6", "4"))
170+
expect_equal(s2$map(c("4", "6", "8", "10")), c("a", NA, "c", NA))
171+
172+
s3 <- scale_colour_manual(
173+
values = c("4" = "a", "6" = NA, "8" = "c"),
174+
na.translate = FALSE, na.value = "x"
175+
)
176+
s3$train(c("8", "6", "4"))
177+
expect_equal(s3$map(c("4", "6", "8", "10")), c("a", NA, "c", NA))
178+
})

0 commit comments

Comments
 (0)