Skip to content

Fix partial match in deprecated guide arguments #5603

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

Merged

Conversation

teunbrand
Copy link
Collaborator

@teunbrand teunbrand commented Dec 21, 2023

This PR to the RC aims to fix a bug uncovered in revdepchecks affecting ~28 packages.

One such example is this:
https://github.com/tidyverse/ggplot2/blob/rc/3.5.0/revdep/problems.md#newly-broken-21

Briefly, title.position gets partially matched to legend.title instead of legend.title.position.
The fix is not to use a list() as theme placeholder but an empty theme() (where we disallow partial matching).

A reprex to reproduce the issue:

library(ggplot2) # RC branch

guide_colourbar(title.position = "top")
#> Error in `mapply()` at ggplot2/R/theme.R:536:3:
#> ! The `legend.title` theme element must be a <element_text> object.

In addition, I found that the reported call for mapply() was uninformative, so I wired some calls to validate_theme().
Now it reports the guide constructor.

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

# Correctly has `legend.title.position` now
x <- guide_colourbar(title.position = "top")
x$params$theme
#> List of 1
#>  $ legend.title.position: chr "top"
#>  - attr(*, "class")= chr [1:2] "theme" "gg"
#>  - attr(*, "complete")= logi FALSE
#>  - attr(*, "validate")= logi TRUE

# New call
guide_colourbar(title.theme = element_rect())
#> Error in `guide_colourbar()`:
#> ! The `legend.title` theme element must be a <element_text> object.

Created on 2023-12-21 with reprex v2.0.2

@teunbrand teunbrand added this to the ggplot2 3.5.0 milestone Dec 21, 2023
@teunbrand teunbrand requested a review from thomasp85 December 21, 2023 09:31
Copy link
Member

@thomasp85 thomasp85 left a comment

Choose a reason for hiding this comment

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

LGTM

@teunbrand teunbrand merged commit 9d1d9c6 into tidyverse:rc/3.5.0 Dec 22, 2023
@teunbrand teunbrand deleted the fix_partial_match_deprecated_args branch December 22, 2023 09:04
thomasp85 pushed a commit that referenced this pull request Feb 23, 2024
* fix partial match bug

* report more informative call
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.

2 participants