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

Guide compatability with ggplot2 >3.4.2 #25

Merged
merged 11 commits into from
Mar 20, 2024
Merged

Conversation

teunbrand
Copy link
Contributor

Hi Charlotte,

Apologies for the cold PR without filing an issue first.
The ggplot2 package has changed the implementation of the guide system, which means that the old S3 will no longer work in ggplot2, see the news file. Because this change isn't fully backwards compatible, guides, including those in ggprism, have broken.

This PR lifts over the old S3 methods for axis drawing so that ggprism can work with the development version of ggplot2.
Tests mostly* run fine in either current CRAN or development version.

* see comments

Best,
Teun

@@ -53,7 +57,7 @@ expect_silent(ggplotGrob(g2))
control <- grab_axis(g1, side = "l")
test <- grab_axis(g2, side = "l")

p + scale_x_discrete(position = "right", guide = "prism_bracket")
# p + scale_x_discrete(position = "right", guide = "prism_bracket")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't sure what this part was supposed to do. It's unfamiliar to me to place an x-axis on the right, and this gives an error with development version of ggplot2.

@csdaw
Copy link
Owner

csdaw commented Jul 6, 2023

Thank for this I'll take a look ASAP

@teunbrand
Copy link
Contributor Author

Hi @csdaw, I've updated the PR for further compatibility with the upcoming ggplot2 version.
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 ggprism get out a fix if necessary.

@teunbrand
Copy link
Contributor Author

This is just a kind reminder that the release is scheduled soon.

@samuel-marsh
Copy link

Hi @csdaw,

Just wanted to add quick ping that it would be great to merge this soon and push to CRAN. I import theme_prism in my package scCustomize and recently ran into check errors due to this upcoming changes in ggplot2 (see tidyverse/ggplot2#5702)

For now I'm going to work in a temp fix so that I can push my release out sooner but would be great to be able to remove that in near future.

Thanks!
Sam

@csdaw
Copy link
Owner

csdaw commented Mar 20, 2024

Sorry for the delay, life things got in the way.

Thanks heaps @teunbrand for all your work 🤩

@csdaw csdaw merged commit 6ffc033 into csdaw:master Mar 20, 2024
1 of 2 checks passed
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.

None yet

3 participants