-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Internal CoordCartesian updates break downstream packages #3561
Comments
I don't think we should add them back... We have enough time to contact all relevant developers and have them fix it. You should focus on pure bugs for now |
Understood. For when that happens, here are the failing packages: Panel params in tests: benchr, biclustermd, seqCAT, Panel params in package code: dabestr, survsup Other packages query the built or rendered object, whose structure has changed with respect to the coordinate system/axes: ggally, ggpol, and mfbvar The largest set of failures are those that depend on plotly, where there is an entire section converting axes from a built ggplot object to a plotly object. This results in quite a few failures for packages that use this functionality. This line appears to be related to the most problems. ggtern breaks because of (at least) internal function changes relating to coordinate systems, which causes several downstream packages to fail. |
Seems ggtern is broken on CRAN. Though there probably is nothing we can do, let me leave a comment here. |
Thanks @yutannihilation! It doesn't look like the package has been updated for 5 years...is it worth me trying to PR a fix? |
Sorry to barge in this conversion; I think ggtern's maintainer switched from github to bitbucket, code there seems not updated since 2018 instead of 2015: https://bitbucket.org/nicholasehamilton/ggtern/src/master/ |
Yes, correct, last commit is December 20, 2018. The ggtern author is aware that an update is necessary: |
The way ggtern is currently set up is problematic: It copies large parts of internal ggplot2 code, modifies it, and then replaces ggplot2's internal functions with those modified versions. This implies that the version of ggtern has to closely track the current version of ggplot2. It's essentially impossible to release a ggtern that survives any major internal development work for ggplot2. I see two ways forward out of this predicament:
|
Thank you all for the information! Sorry, I saw your conversation on #2540, but just forgot it.
Yeah, it's problematic. I found it breaks other extension packages... |
But, then, it seems I commented on a wrong issue. It seems the problems on ggtern are more general ones. Can we close this issue? As I commented on #3871 (comment), I just wondered if we still need to look into the v3.3.0 milestones that are left open. If necessary, I'll open a new issue to discuss how we should solve the breakage of ggtern. |
Many of the packages in @paleolimbot's comment from October 10, 2019 are still broken, e.g. ggally, ggpol, mfbvar. Where they informed at the time that this change was coming? There's only so much we can do if people depend on undocumented internal data structures and then don't stay on top of the ongoing ggplot2 development. |
I went through one round of revdeps for scale-related failures and did notify some downstream developers, but I don't think that I ended up notifying this specific set of developers (probably my mistake). I'll do this today (and offer to fix those that are easy to). |
It's also my fault that I didn't check if this issue was resolved until the release while it's me who marked this as a milestone. Sorry...
This one? #3565 |
All packages with failing checks were notified a month in advance and have had plenty of time to fix their code |
Ah, you did it by email. Thanks, I got it. |
I opened an issue in ggally since it has a large number of downloads, and I think given the email to the other authors, we've done enough to close this particular issue (I'll open another for ternary diagrams if y'all haven't already) |
In #3356, the
panel_params
generated byCoordCartesian
were changed so thatpanel_params$x
andpanel_params$y
are scale-like objects. Internally we usepanel_params$x.range
andpanel_params$y.range
(the x and y scale limits), so these were kept for backward compatibility. This change broke a number of downstream packages (e.g., #3436 and multiple reverse dependency failures). We should consider adding back the following elements to ease this transition:x.labels
y.labels
x.major.source
y.major.source
x.minor.source
y.minor.source
Thoughts on this @thomasp85? Also tagging @cpsievert since he was the first one to notice this change.
This is where one would add these parameters back in:
ggplot2/R/coord-cartesian-.r
Lines 206 to 211 in 9b667b9
The text was updated successfully, but these errors were encountered: