Skip to content

Theme palettes warn about options - #6334

Merged
teunbrand merged 4 commits into
tidyverse:mainfrom
teunbrand:theme_check_palettes
Mar 25, 2025
Merged

teunbrand merged 4 commits into
tidyverse:mainfrom
teunbrand:theme_check_palettes

Conversation

@teunbrand

@teunbrand teunbrand commented Feb 17, 2025 •

Copy link
Copy Markdown
Collaborator

This PR aims to fix #6333.

Briefly, theme() checks whether colour palettes have been set simultaneously in the options and emit a warning when they are.

RE: my earlier comment:

Mechanically, would you propose to detect this situation whenever theme() is called, or when scale_colour_*() is called?

I think theme() is the more appropriate place because:

  • theme() has access to the theme palette and the options
  • scale_colour_*() never gets access to the theme, and the alternative would be ScalesList$set_palettes(), at which point it would be too late the change the entire scale rather than just the palette.
  • I imagine options() is more used in a global fashion than theme(). The case that we'd be missing now is if we do a theme_set() with a palette before options().
  • An alternative is to do this in check_theme(), which runs every render.

Using theme without options is silent:

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

p <- ggplot(mpg, aes(displ, hwy, colour = drv)) +
  geom_point()

p + theme(palette.colour.discrete = c("dodgerblue", "orange", "orchid"))

Using options without theme settings is silent:

options(ggplot2.discrete.colour = c("red", "yellow", "limegreen"))
p

But using both emits a warning.

p + theme(palette.colour.discrete = c("dodgerblue", "orange", "orchid"))
#> Warning: The `options('ggplot2.discrete.colour')` setting is incompatible with the
#> `palette.colour.discrete` theme setting.
#> ℹ You can set `options(ggplot2.discrete.colour = NULL)`.

Created on 2025-02-17 with reprex v2.1.1

Comment thread R/theme.R
validate = TRUE) {
elements <- find_args(..., complete = NULL, validate = NULL)

elements <- fix_theme_deprecations(elements)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I've separated this in its own function as all these deprecations became somewhat distracting in ascertaining what theme() actually does.

@thomasp85 thomasp85 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Merge branch 'main' into theme_check_palettes

# Conflicts:
#	R/theme.R
#	tests/testthat/test-theme.R
@teunbrand
teunbrand merged commit f34ad83 into tidyverse:main Mar 25, 2025
@teunbrand
teunbrand deleted the theme_check_palettes branch March 25, 2025 13:51
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.

Should theme(palette.* = ) raise a warning when options(ggplot2.* = ) is set?

2 participants