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

unexpected behavior when setting named vector for default colour/fill #5013

Open
mjsmith037 opened this issue Oct 14, 2022 · 4 comments · May be fixed by #5954
Open

unexpected behavior when setting named vector for default colour/fill #5013

mjsmith037 opened this issue Oct 14, 2022 · 4 comments · May be fixed by #5954

Comments

@mjsmith037
Copy link

mjsmith037 commented Oct 14, 2022

First, thanks for adding this functionality to ggplot2 - it has been enormously useful in my day-to-day coding. Now on to the issue:

When setting a default color palette using ggplot2.options.discrete.colour or ggplot2.options.discrete.fill, the plotting function will only pull a subset of the submitted vector of length equal to the number of unique colours needed. This works fine for unnamed color vectors, however it has undesirable behavior when the relevant named elements occur outside of this subset.

library(ggplot2)
options(ggplot2.discrete.fill=c("4"="red", "8"="blue", "6"="green"))
data(mpg)

# removing 5 here for a cleaner plot; works as expected
ggplot(mpg[mpg$cyl != 5,]) +
  aes(x=hwy, fill=factor(cyl)) +
  geom_density()

# note that even though we have a colour named "6" in the above vector, ggplot2
# does not find it and instead uses the NA fill
ggplot(mpg[mpg$cyl != 5 & mpg$cyl != 8,]) +
  aes(x=hwy, fill=factor(cyl)) +
  geom_density()

Created on 2022-10-14 by the reprex package (v2.0.1)

@smouksassi
Copy link

did you want to use scale fill manual ? this is not how scale discrete is to be used

@mjsmith037
Copy link
Author

By "is to be used," do you mean default discrete scales should not be named? I guess I might come to understand a principled stand on this, however it would avoid confusion if these scales were then unname()d (ideally with a warning) on the back end. Documentation could also go a long way toward specifying desired use. As it is, I found the use I outlined above intuitive outside of this particular instance of mismatch.

@yutannihilation
Copy link
Member

Thanks for catching. I too feel it's a bit confusing at least that the behavior is different from scale_fill_manual()...

library(ggplot2)

ggplot(mpg[mpg$cyl != 5 & mpg$cyl != 8,]) +
  aes(x=hwy, fill=factor(cyl)) +
  geom_density() +
  scale_fill_manual(values = c("4"="red", "8"="blue", "6"="green"))

Created on 2022-11-05 with reprex v2.0.2

@teunbrand
Copy link
Collaborator

The issue would be resolved if we remove the [seq_len(n)] in the line below:

type_list[[i]][seq_len(n)]

But perhaps the broader question is whether we want to support named palettes in the options?

@teunbrand teunbrand linked a pull request Jun 21, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants