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

First argument to scales is not longer the name of the scale #5535

Closed
eliocamp opened this issue Nov 22, 2023 · 4 comments · Fixed by #5583
Closed

First argument to scales is not longer the name of the scale #5535

eliocamp opened this issue Nov 22, 2023 · 4 comments · Fixed by #5583
Labels
bug an unexpected problem or unintended behavior scales 🐍
Milestone

Comments

@eliocamp
Copy link
Contributor

This seems to be a breaking change in the current development version.

In ggplot2 version 3.4.4 (latest tag), scale_color_brewer("year") would set "year" as the name of the scale:

library(ggplot2)
packageVersion("ggplot2")
#> [1] '3.4.4'
ggplot(mpg, aes(displ, hwy)) +
  geom_point(aes(colour = factor(year)), size = 5)  +
  scale_color_brewer("year")

In current dev version, this doesn't work:

library(ggplot2)
packageVersion("ggplot2")
#> [1] '3.4.4.9000'
ggplot(mpg, aes(displ, hwy)) +
  geom_point(aes(colour = factor(year)), size = 5)  +
  scale_color_brewer("year")

@teunbrand
Copy link
Collaborator

teunbrand commented Nov 22, 2023

I think the deprecation of the scale_name argument now lets that deprecated argument become the first argument that ... now feeds into. I agree with you that this isn't ideal. There are three ways I can see this solved:

  1. Make the first argument to every scale explicitly the name argument.
  2. Move the deprecated scale_name argument to one of the last arguments. I don't know how many scale extensions use positional versus named argument declarations and if this will be harmful. edit: it seems pretty common to match by position.
  3. Explicitly declare scale_name = lifecycle::deprecated() in every call to the constructor so that feeding ... gets name back as the first argument.

I'm not sure which approach will be preferable.

Until that is figured out, you can fall back to using explicit scale_color_brewer(name = "year") or implicit scale_color_brewer(,"year").

@eliocamp
Copy link
Contributor Author

I like 1 :D

@jan-glx
Copy link
Contributor

jan-glx commented Nov 25, 2023

Until this is figured out one can fall back to install an older version remotes::install_github("tidyverse/ggplot2@67bb3bbea8f51666b13d9ca30108d97e898c1ecd") . ( Updating my whole (analysis) code base is not an option for me atm. )

I am probably wrong here, but it seems to be that neither of the three options are backwards compatible in all cases?
It further seems that there are two separate issues: scale_* functions (which did not allow to specify scale_name) and *_scale functions....

@teunbrand teunbrand added scales 🐍 bug an unexpected problem or unintended behavior labels Nov 29, 2023
@teunbrand teunbrand added this to the ggplot2 3.5.0 milestone Nov 29, 2023
@teunbrand
Copy link
Collaborator

teunbrand commented Dec 5, 2023

Yeah option 1 seems to be most sensible.
To prevent gnarly merge conflict, probably best to finish off #5566 first.

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 scales 🐍
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants