Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quirky NA-handling of geom_ribbon() #6243

Open
teunbrand opened this issue Dec 16, 2024 · 2 comments
Open

Quirky NA-handling of geom_ribbon() #6243

teunbrand opened this issue Dec 16, 2024 · 2 comments
Labels
bug an unexpected problem or unintended behavior

Comments

@teunbrand
Copy link
Collaborator

Now that we're allowing gradients in geom_ribbon() and derivatives, the following caught my attention.

In the example below, we simply want to vary the gradient along x, so top and bottom gradients should match.
However, the gradient is misplaced because the second group has a missing value (note it starts with dark blue in the middle).

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

df <- data.frame(
  x = c(1:3, 1:3),
  ymin = c(1, 1, 1, NA, 2, 2),
  ymax = c(1.5, 2, 1.5, 2.5, 3, 2.5),
  group = c(1,1,1,2,2,2)
)

p <- ggplot(df, aes(x, ymin = ymin, ymax = ymax, group = group, fill = x)) +
  scale_fill_viridis_c()

# Wrong
p + geom_ribbon()

The gradients are drawn correctly if missing values are removed properly.

p + geom_ribbon(na.rm = TRUE)

Created on 2024-12-16 with reprex v2.1.1

I think there are two things wrong with this when na.rm = FALSE:

  • There is no warning about missing values, as you would expect with any other geom
  • It doesn't actually remove the missing data, causing the misalignment of gradients.

I traced this decision to a remedy against #1549.

@clauswilke
Copy link
Member

In either case, this seems to be an incorrect implementation of na.rm. In all other geoms it has no effect other than suppressing a warning. But here it seems to alter the data.

From the geom_point() documentation (as one example):
Image

@teunbrand
Copy link
Collaborator Author

Agreed. If #1549 it intended to not fill over missing values, we could separate groups of NA-gaps.

@teunbrand teunbrand added the bug an unexpected problem or unintended behavior label Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

2 participants