Compatibility upcoming ggplot2 version

[teunbrand]

Hi there,

We've been preparing a new major release for ggplot2 and found an issue during a reverse dependency check.
We see a series of test failures with plotting functions, but it isn't straightforward for us to debug what is going wrong.

You can test your code with the development version of ggplot2 by installing it as follows:

# install.packages("pak")
pak::pak("tidyverse/ggplot2")

We aim to release the new ggplot2 version soon. It appears that mlr3spatiotempcv had a recent update which might be why this wasn't detected in earlier rounds of reverse dependency checks, so we apologise for the short notice.

Best wishes,
Teun

[pat-s]

@teunbrand

Thanks for the info. I did a quick check and found the following:

Check on 'p3' failed: Must be of type 'list', not 'patchwork/ggplot2::ggplot/ggplot/ggplot2::gg/S7_object/gg'

is there an official migration guide for this new list enforcement? Or was it an oversight to not support {patchwork} objects anymore?

[teunbrand]

It is a bit hard to grasp the exact context from your message, but if you expect a list type and it is not, the following might be a related issue with solutions: tidyverse/ggplot2#6498.

The latest patchwork release was tested and should be compatible.

[pat-s]

My point is: nothing has changed in the plotting logic in my package. There are breaking changes in a new {ggplot2} version which don't natively allow {patchwork} objects anymore, or at least in the same form, as before that update.

So as a package author facing a breaking change, it would be good to know/read about 1. what has changed 2. why that happened and ideally have some migration instructions somewhere without having to fiddle it all out myself (as I am not responsible for the breaking change :))

If this is a new case that is not yet known, it is so now - and you can also reproduce it by running the tests of this package. Again, happy to make changes but ideally the digging work on how/where/why is not entirely on me.

[teunbrand]

There are breaking changes in a new {ggplot2} version which don't natively allow {patchwork} objects anymore,

I'm not sure what you mean about (dis)allowing patchwork objects.

I did a quick check and found the following:

The message you referred to, I find in tests like these:

mlr3spatiotempcv/tests/testthat/test-2-autoplot.R

Line 26 in f0f3123

expect_list(p3)

Where you're making assumptions about the structure about a patchwork object (namely it being a list). That assumption no longer holds. The patchwork docs mention that 'patchwork object' is returned, with no mention of a list.

ideally the digging work on how/where/why is not entirely on me.

This expectation is reasonable, and I would've notified and/or helped identify the exact cause if this weren't discovered so late in the release process after multiple prior rounds of reverse dependency checks (as I have for plenty of other packages).

[pat-s]

I'm not sure what you mean about (dis)allowing patchwork objects.

The error clearly states that the return can no longer be a patchwork object (for whatever reason). This is what I meant with "disallowed".

Where you're making assumptions about the structure about a patchwork object (namely it being a list). That assumption no longer holds.

Yes, that's what the error message says 🙃️ The question is: why is that, what should it be now, etc. And ideally this information would (have) come in a migration note as it seems that the return of a function in ggplot2 changed, which eventually caused this.

Anyhow, I am not sure we fully understand each other here, I might be bad in explaining.
I should just look into it actively instead of asking for migration support, its likely a small fix and not worth the long discussion. Thanks for your initial note about this in the first place.

so late in the release process after multiple prior rounds of reverse dependency checks

Hm, but why would that close out the act of helping then? Even if, this just means the process order for the release is likely suboptimal but it shouldn't result in "no help" for the affected dependencies. And I am not sure I can follow a time-pressured release process for an R package which includes breaking changes (and is not necessarily tackling important bug fixes).

[teunbrand]

The error clearly states that the return can no longer be a patchwork object (for whatever reason).

I'm not following you here. The error after executing what code? I have seen no error message that something cannot be a patchwork object.

The question is: why is that, what should it be now, etc. And ideally this information would (have) come in a migration note

You may find this explained in the issue I've linked to earlier.

Hm, but why would that close out the act of helping then?

In general, I'm happy to help, as I've done for many packages and would have done if we detected this problem earlier, but we can't keep postponing the release as new issues accrue even within 2 weeks notice. Also, posting a notification in advance and discussing the issue is providing help, even if it is not a PR that fixes the issue directly.