Internal CoordCartesian updates break downstream packages

[paleolimbot]

In #3356, the panel_params generated by CoordCartesian were changed so that panel_params$x and panel_params$y are scale-like objects. Internally we use panel_params$x.range and panel_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:

https://github.com/tidyverse/ggplot2/blob/9b667b94df789bed4e4fa2ab16c5d428c8282f6c/R/coord-cartesian-.r#L206-L211

[thomasp85]

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

[paleolimbot]

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.

[yutannihilation]

Seems ggtern is broken on CRAN. Though there probably is nothing we can do, let me leave a comment here.

• https://cran.r-project.org/web/checks/check_results_ggtern.html
• https://github.com/nicholasehamilton/ggtern/issues/41

[paleolimbot]

Thanks @yutannihilation! It doesn't look like the package has been updated for 5 years...is it worth me trying to PR a fix?

[teunbrand]

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/

[clauswilke]

Yes, correct, last commit is December 20, 2018.

The ggtern author is aware that an update is necessary: https://github.com/tidyverse/ggplot2/issues/2540#issuecomment-570723572 In fact, some of the theme changes I made for 3.3.0 where specifically prompted by his request.

[clauswilke]

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:

1. Take the most recent ggplot2 version that works with the current ggtern, literally copy everything needed into ggtern, and then rename all relevant S3 classes and functions so the namespaces don't clash. This is essentially a fork of ggplot2 for ternary plots. I don't think it's a great solution, but it's something that can be done in the short term.

2. Rework ggplot2 so it can support ggtern (or something similar) via the regular, published extension API. This should be our long-term goal, but it requires some substantial changes that we will have to make very carefully and deliberately. At that point, we'd almost certainly be talking about at least a ggplot2 4.0.0.

[yutannihilation]

Thank you all for the information! Sorry, I saw your conversation on #2540, but just forgot it.

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.

Yeah, it's problematic. I found it breaks other extension packages...

https://github.com/yutannihilation/gghighlight/issues/135

[yutannihilation]

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 https://github.com/tidyverse/ggplot2/issues/3871#issuecomment-598681750, 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.

[clauswilke]

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.

[paleolimbot]

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).

[yutannihilation]

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...

scale-related failures

This one? https://github.com/tidyverse/ggplot2/issues/3565

[thomasp85]

All packages with failing checks were notified a month in advance and have had plenty of time to fix their code

[yutannihilation]

Ah, you did it by email. Thanks, I got it.

[paleolimbot]

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)