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

Test for absence/presence of guide differently #89

Merged
merged 2 commits into from
Jan 19, 2024
Merged

Test for absence/presence of guide differently #89

merged 2 commits into from
Jan 19, 2024

Conversation

teunbrand
Copy link
Contributor

Hi Claus,

It is me again, running down reverse dependency checks. I'm posting this because the prospective ggplot2 3.5.0 would break a test in ggridges.

The change is that a plot now always has guide boxes (though they are zero grobs if empty). Hence, testing for presence/absence of a guide by checking the gtable for the 'guide-box' name no longer works. This PR uses the new get_guide_data() to test for a legend's presence or absence.

For completion; to test the code changes with the release candidate, you can install it with the code below:

remotes::install_github("tidyverse/ggplot2", ref = remotes::github_pull("5592"))

The release of ggplot2 3.5.0 is scheduled for the 12th of February. The progress of the release can be tracked in tidyverse/ggplot2#5588. I hope that this PR might help ggridges get out a fix if necessary.

@clauswilke
Copy link
Collaborator

Thanks, this looks rather straightforward. Could you just add a little comment, something like "Legend structure changed with ggplot2 3.5" or similar, so the conditional testing is properly documented for the future? Ideally on both tests.

@teunbrand
Copy link
Contributor Author

You got it 👍

@clauswilke clauswilke merged commit 648001c into wilkelab:master Jan 19, 2024
5 checks passed
@teunbrand teunbrand deleted the ggplot2_3.5.o branch January 19, 2024 07:52
@clauswilke
Copy link
Collaborator

This is now on CRAN.

@teunbrand
Copy link
Contributor Author

Thank you Claus!

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