Revdep: checking for classes

[teunbrand]

Problem

This issue emerged from a recent revdepcheck (de2715d), and affects an estimated 91 reverse dependencies. The issue is that tests based on ggplot2's classes fail because we transitioned to S7 classes.

It includes tests for the class itself

expect_equal(class(p), c("gg", "ggplot"))

But also tests that relate to class structure like so:

expect_length(p, 11)
expect_type(p, "list")

The following case works for now, but we might remove the manual "ggplot" S3 class in the future. It is not the recommended way of testing.

expect_s3_class(p, "ggplot")`

This pattern may hold for other classes, like mapping, labels, margin or elements too.

Solution

There are several possibilities for adjusting the test.

• The preferred way is to use expect_true(is_ggplot(p)). The is_ggplot() function is internally consistent regardless of whether the ggplot object is the old or new class.
• Alternatively, you can use expect_true(inherits(p, c("ggplot", "ggplot2::ggplot"))), which requires a match to one of the two classes.
• Lastly, you can use expect_s7_class(p, class_ggplot), but that will only be stable after we've released the new version. It is not suitable for backward compatibility.

Unfortunately, this is not something that is straightforward to fix in ggplot2, so this probably will have to be updated in the reverse dependencies.

[teunbrand]

Informed by email, as there is no linked GitHub page in the DESCRIPTION files:

• ASRgenomics
• bdsm
• benchr
• calibmsm (received response)
• carbonr (out of office)
• CausalImpact
• cmcR
• conquestR (received response)
• fChange
• gfoRmulaICE (bounced)
• ggbump (received response)
• ggstream (received response)
• miRetrieve
• mshap
• mxnorm
• NiLeDAM (received response)
• rbioacc (https://github.com/tidyverse/ggplot2/issues/6498#issuecomment-2963298088)
• rifreg
• rPBK
• RVenn
• Rwtss
• SOMbrero (received response)
• tidyEdSurvey
• ecocbo

[teunbrand]

Mistakingly classified a related problem for this exact problem:

• biclustermd
• canvasXpress
• ggblend
• listdown
• ggparalllel
• scplot
• synthpop

[teunbrand]

Not informed:

• clifro (archived)

[njlyon0]

Thanks for letting us know! For those of us who got this issue because of a unit test checking for object class (E.g., expect_equal(class(p), c("gg", "ggplot"))), will the following tweak work based on your planned release?

expect_true( all( c("gg", "ggplot") %in% class(p) ) )

Also, what will the new version number for ggplot2 be? I'd like to identify it in my NEWS file.

[teunbrand]

will the following tweak work

I think it will technically work, but it isn't preferred because we intend to remove the "gg" and "ggplot" classes in the future. These class names are a legacy of the S3 system and we had to keep them to minimise breakage in other packages. For that same reason we also don't recommend expect_s3_class(p, "ggplot") You can use one of the following:

• expect_true(is_ggplot(p))
• expect_true(inherits(p, c("ggplot", "ggplot2::ggplot")))
• expect_s7_class(p, class_ggplot) (but only after the update is released)

Also, what will the new version number for

We intend to number it version 4.0.0

[john-harrold]

Howdy Folks. I'm responding to the issue here: https://github.com/john-harrold/formods/issues/36

I'm using the class to test whether a ggplot object has been built successfully:

library(ggplot2)
library(testthat)

p = ggplot()
expect_equal(class(ggplot2::ggplot_build(p)), "ggplot_built")

Can someone tell me how I should do this in the new version of ggplot. Preferably something that would be backwards compatible :)

[teunbrand]

how I should do this in the new version of ggplot

The ggplot_built class is slightly trickier because we don't have an is_ggplot_built() tester (but maybe we should?). Instead you can use the following:

expect_true(inherits(p, c("ggplot2::ggplot_built", "ggplot_built")))

[john-harrold]

Thankyou @teunbrand. It looks like this works for me. I'll submit a new version of formods to CRAN over the weekend with this change.

Since I'm here I waned to thank all the folks working on ggplot2 and also the tidyverse. It's an amazing amount of work.

[njlyon0]

Ditto what John said on both counts!

[virgile-baudrot]

Hi @teunbrand thank for the e-mail. i'll try to fix it for rbioacc within 2 weeks, issue rbioacc. I'll give you feedback here, when it's done and resubmit to CRAN. Best

[billdenney]

Thanks for the hard work to improve ggplot2!

I'm working on the fix to nlmixr2plot, and the class check changes are straight-forward.

We have a method for addition that is now broken. Generally it looks like our_object + ggplot2::xlab("new label") and that returns NULL now. Do you have a pointer for how to make addition with the new S7 ggplot2 object work where we want to add a ggplot2 object to our object?

[teunbrand]

We have a method for addition that is now broken

I'm also struggling with that myself, I've made a separate issue to discuss: https://github.com/tidyverse/ggplot2/issues/6504

[john-harrold]

I'm doing some testing and I think this also breaks plotly. The code below works fine on the version of ggplot2 currently on CRAN but breaks with version 3.5.2.9001 installed off of github.

library(ggplot2)
library(plotly)
df = data.frame(x=c(1:10), y=c(1:10))
p   = ggplot(data=df) + 
      geom_point(aes(x=x, y=y)) + 
      xlab("x") + ylab("y")
ply = ggplotly(p, source="my-figure")

The error I get is:

Error in pm[[2]] : subscript out of bounds

[teunbrand]

I think this also breaks plotly.

Have you tried installing the latest version from CRAN? I recall this is indeed the old error that was thrown by plotly

[john-harrold]

I'm sorry you're correct. I thought I had updated everything already.

[AlphaPrime7]

For 3.5 the syntax seems to be is.ggplot()

[teunbrand]

Yes we replaced is.ggplot() with is_ggplot() in 3.5.2.