Skip to content

Conversation

teunbrand
Copy link
Collaborator

@teunbrand teunbrand commented Jun 19, 2024

This PR aims to fix #5929.

Briefly, there are 3 situations by which a value can become NA in a discrete scale.

  1. The data value is NA.
  2. The data value does not match the scale limits.
  3. The data value is mapped to NA because a palette value is NA.

Currently, we cannot distinguish between situation (2) and situation (3).
This PR ensures that situation (1) and (2) are laundered through the na.translate/na.value mechanism, but (3) is not because we could assume (3) is intentional by users (see issue).

As a demo, notice that 'e' gets NA due to (3) and is subsequently removed, while 'r' gets na.value due to (2).

devtools::load_all("~/packages/ggplot2/")
#> ℹ Loading ggplot2

ggplot(mpg, aes(displ, hwy, colour = fl)) +
  geom_point() +
  scale_colour_manual(
    values = c("tomato", "dodgerblue", NA, "limegreen"),
    limits = function(x) setdiff(x, "r"),
    na.value = "black"
  )
#> Warning: Removed 8 rows containing missing values or values outside the scale range
#> (`geom_point()`).

Created on 2024-06-19 with reprex v2.1.0

Comment on lines +982 to +983
pal <- c(pal, na_value)
pal_match <- pal[match(as.character(x), limits, nomatch = length(pal))]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is the trick to discrimating (2) and (3). In case (2) we get nomatch'ed to the last palette value, which we set to the na.value. This leaves (3) as-is.

Comment on lines +985 to +986
if (!is.na(na_value)) {
pal_match[is.na(x)] <- na_value
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We no longer need to translate is.na(pal_match) because we do so through the (lack of) matching above

Copy link
Member

@thomasp85 thomasp85 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@teunbrand teunbrand merged commit c679d43 into tidyverse:main Jul 12, 2024
@teunbrand teunbrand deleted the na_palettes branch July 12, 2024 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ggplot2 ignores the user's input for transparency via NA's in scale_fill_manual()

2 participants