deal with old and new guides #54
Conversation
| # # To use new guide system activate the line below | ||
| # pair$guide$draw(theme, params = pair$params) | ||
| # # And deactivate the line below up until the `} else {` line | ||
| if (inherits(pair$params$angle, "waiver")) { | ||
| pair$params$angle <- NULL | ||
| } | ||
| guide_gengrob.ggside_axis(pair$params, theme) |
There was a problem hiding this comment.
Using the new system would result in some visual changes related to the spacing around axes.
I didn't fully understand where this change would come from, so I retained the old drawing method.
If you want to switch to the new guide system, you can use pair$guides$draw() instead.
There was a problem hiding this comment.
I appreciate that you have taken the time to look into correcting these. I have been working on bringing ggside up to speed with ggplot2 3.5.0 on the dev branch. I do not know which branch you were testing against, but my guess would be the main branch.
The dev branch should be compatible with ggplot2, but it isn't in a state where I'm ready to release. I've been putting a majority of my refactoring into dev-refactor which will eventually be merged to dev and then main.
I think in the future I will follow ggplot2's coding strategy and leave main as the development branch that is out of sync with CRAN.
I have switched to the new guide system where relevant in ggside and just ran ggside's tests against
remotes::install_github("tidyverse/ggplot2", ref = remotes::github_pull("5592"))Only about 12 tests out of 275 had failures in that there was more spacing given to the Y axis title when the axis was plotted on the right. I for one can live with these changes 👍
Thanks again for your diligent work!
|
Apologies, I hadn't noticed the dev branches and all the work you had put in already. That is amazingly pro-active.
This might be expected as the previous right y-axis title's If this PR is redundant, please feel free to close :) |
We've settled on a release date for ggplot2 3.5.0 at February 12th. If it is unfeasible to get your big feature release out before then, might I gently and kindly put the notion out there that a small, targeted hotfix would be beneficial to minimise breakages? (I know this is ggplot2's fault for causing the breakages in the first place) If you'd like me to amend or retarget this PR to another branch, don't hesitate to ask :) |
|
This is just a kind reminder that the release is scheduled soon. |
|
I have just submitted my package version, sorry for the radio silence, I wasn't sure if I could release a new version since my package depended on 3.5.0 to have CI tests pass as expected. I am going to close this PR. Thanks again for everything! |
TL;DR: This PR aims to fix #46.
Hi Justin,
We have been preparing a new release of ggplot2 and during a reverse dependency check, it became apparent that the prospective ggplot 3.5.0 would break ggside.
Given that other packages also import ggside we felt it was prudent to help out to transition to the new version.
The culprit is that ggplot2 has changed the guide system, see tidyverse/ggplot2#4879.
This PR handles the new guide infrastructure and should work with both the current and the development version of ggplot2.
To test the code changes with the release candidate, you can install it with the code below:
The release of ggplot2 3.5.0 is scheduled at the end of January / early Februari. The progress can be tracked in tidyverse/ggplot2#5588.
We are hoping that this PR might help ggside be ready around that time to soften the impact on downstream packages.