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

Scale_*_viridis_b not using full width of palette #4372

Closed
gregleleu opened this issue Mar 16, 2021 · 10 comments · Fixed by #4737
Closed

Scale_*_viridis_b not using full width of palette #4372

gregleleu opened this issue Mar 16, 2021 · 10 comments · Fixed by #4737
Labels
scales 🐍 visual change 👩‍🎨 Rendering change that will affect look of output

Comments

@gregleleu
Copy link
Contributor

gregleleu commented Mar 16, 2021

When using scale_(fill/colour)_viridis_b, the colors don't go all the way to the end of the palette, i.e. very-yellow yellow).
Is it intended to do that? How to change this behavior if it is?

library(ggplot2)

ggplot(data = ggplot2::diamonds) +
  geom_bar(aes(x = cut, fill = depth, group = depth)) +
  scale_fill_viridis_b(n.breaks = 4)

image

scales::show_col(
  viridis::viridis_pal()(4)
)

Full palette (4 steps)
image

@yutannihilation
Copy link
Member

  scale_fill_viridis_b(n.breaks = 7)

Did you mean n.breaks = 4 here?

@gregleleu
Copy link
Contributor Author

@yutannihilation yes, thanks, fixed it (I started with an example with 7 breaks, but changed to 4 to make it simpler)

@yutannihilation
Copy link
Member

It's not that the values are binned and then mapped to a discrete palette; the palette is still continuous and trained by the original values.

library(ggplot2)
library(patchwork)

range(diamonds$depth)
#> [1] 43 79

# a similar range of values
d <- data.frame(x = 40:80, y = 1)

p <- ggplot(d) +
  geom_col(aes(x, y, fill = x))

p1 <- p + scale_fill_viridis_c(breaks = 4:7 * 10 + 5)
p2 <- p + scale_fill_viridis_b(n.breaks = 4)

p1 / p2

Created on 2021-03-17 by the reprex package (v1.0.0)

@gregleleu
Copy link
Contributor Author

@yutannihilation thanks for looking into it.

I'm not sure I understand the cases where this is the better behavior.
I was under the impression binned scales were added so ggplot could do the binning "on-the-fly" and we wouldn't have do to it in the data beforehand. If so the fact that we don't have the same colors in the end is more often a bug than a feature. But also that's why I suggested having the option between the behaviors for the people who need what you describe.

@clauswilke
Copy link
Member

I think I'm with @gregleleu here and I would propose reopening the issue. I understand how the color scale gets mapped to the data values currently, and I agree it's a reasonable way of doing it, but stretching the entire range of color values over the existing bins such that the endpoints align would be similarly reasonable. There could be an option in binned_scale() to toggle between these two behaviors.

@yutannihilation
Copy link
Member

Okay, then let's reopen. I have no strong opinion here.

@thomasp85 thomasp85 added scales 🐍 visual change 👩‍🎨 Rendering change that will affect look of output labels Mar 24, 2021
@modche
Copy link

modche commented Feb 18, 2022

Hi there. Is there any progress? I totally agree with @gregleleu and @clauswilke, it would be nice if the full range can be used in the binned version of color maps!

@heike
Copy link

heike commented Feb 18, 2022

@modche the fantastic thing about open source development is that everybody can pitch in with pull requests!

@gregleleu
Copy link
Contributor Author

Added a PR to resolve this, adding a parameter to scale_*_viridis_b functions to switch the palette between gradient_n_pal and binned_pal.
Honestly, I don't understand why the gradient_n_pal version does not give the same output in the first place. Maybe down in the working of BinnedScale... ggplot2 is not an easy package to contribute to (even though it is fantastically open-source / I was going to write another passive-aggressive message, but I figured a PR would be more interesting)

@modche
Copy link

modche commented Feb 21, 2022

Thanks, @gregleleu, for your efforts. And, yes, @heike, a PR is certainly a good way to proceed here... I just was not able to figure out how to do this in this case. But, now, we have a suggestion. Would be nice to use the brilliant yellow from viridis an all scale_*_binned commands! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scales 🐍 visual change 👩‍🎨 Rendering change that will affect look of output
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants