Identify revdep issues

[hadley]

There are 229 new failures, listed below. This is too many, and suggests that we've accidentally broken something important (hopefully we've only broken 1 or 2 things that are used heavily not a bunch of small things). The first step is to find the common themes in the failures.

@tidyverse/ggplot2 can you please let me know if you have time this week to read some fraction of the failure reports (in <revdep/README.md>) and report back? I'll split the packages up over everyone who's willing to help.

Googlesheet of failures at https://docs.google.com/spreadsheets/d/1UZIFwt3j3XjnRGOMNJZvpal_CAj7Hz8aNXJt41m3JjM/edit#gid=0 — when you start exploring a failure, first put your name in the "who" column to make sure multiple people don't try understanding the same package at the same time.

[hadley]

Another common problem: looks like we accidental broke geom look up in qplot(). So any issue with could not find function "geom_text" (or similarly) is likely to be #2586

[clauswilke]

I see several warnings about an openMP clash, e.g.:

    > # Use image as plot background
    > p <- ggplot(iris, aes(x = Sepal.Length, fill = Species)) + geom_density(alpha = 0.7)
    > ggdraw() +
    +   draw_image("http://jeroen.github.io/images/tiger.svg") +
    +   draw_plot(p + theme(legend.box.background = element_rect(color = "white")))
    OMP: Error #15: Initializing libomp.dylib, but found libomp.dylib already initialized.
    OMP: Hint: This means that multiple copies of the OpenMP runtime have been linked into the program. That is dangerous, since it can degrade performance or cause incorrect results. The best thing to do is to ensure that only a single OpenMP runtime is linked into the process, e.g. by avoiding static linking of the OpenMP runtime in any library. As an unsafe, unsupported, undocumented workaround you can set the environment variable KMP_DUPLICATE_LIB_OK=TRUE to allow the program to continue to execute, but that may cause crashes or silently produce incorrect results. For more information, please see http://www.intel.com/software/products/support/.

This seems to happen with packages that work with images, and it may be a problem with the testing environment rather than indicating a real problem?

[batpigandme]

Seeing quite a few (38 based on just a simple raw count):

variables…have non standard format: "~foo".  Please rename the columns or make a new column.

Reprex of one from GGally

library(GGally)
data(diamonds, package = "ggplot2")
diamonds.samp <- diamonds[sample(1:dim(diamonds)[1], 1000), ]
pm <- ggpairs(diamonds.samp,
  columns = 5:7,
  mapping = ggplot2::aes(color = color),
  upper = list(continuous = "cor", mapping = ggplot2::aes_string(color = "clarity")),
  lower = list(continuous = "cor", mapping = ggplot2::aes_string(color = "cut")),
  title = "Diamonds Sample"
)
#> Error in make_ggmatrix_plot_obj(wrapp(sub_type, funcArgName = sub_type_name), : variables: "x" have non standard format: "~depth", "~color".  Please rename the columns or make a new column.variables: "colour" have non standard format: "~depth", "~color".  Please rename the columns or make a new column.

Created on 2018-05-14 by the reprex package (v0.2.0).

[hadley]

@batpigandme Is it possible all those errors come downstream from GGally? Would you mind trying to reproduce one or two more and seeing if make_ggmatrix_plot_obj() is in the traceback?

[batpigandme]

Nailed it!

At least 12 have make_ggmatrix_plot_obj() directly in their error reports https://raw.githubusercontent.com/tidyverse/ggplot2/master/revdep/problems.md
and then I found a few others in their vignettes and examples, and so far they've all traced to make_ggmatrix_plot_obj(). And, it looks like that's ~ the same thing when there are ggpairs()-related failures, too.

[thomasp85]

There's a bunch of errors that happens because eval_tidy doesn't allow duplicate column names. I would argue that this is fair and it is up to the package developers to clean their data, so I won't open an issue here. I'll mark the relevant package in the sheet

[hadley]

@lionel- We also need a paragraph for the breaking changes section describing how the internal structure of aes() has changed.

[karawoo]

I don't think so. There are also a number of ggplot2::calc()/raster::calc() issues.

[batpigandme]

There are also seven packages with undefined columns selected errors — is that due to a change in tidyselect?

[hadley]

forecast::autolayer() will be resolved by forecast developers since @mitchelloharawild requested it

How many raster::calc() conflicts are there? If there's a lot we might need to change calc() to something else.

Can someone give me a traceback for 2-3 undefined columns selected errors?

[karawoo]

I count 10 packages with raster::calc() conflicts: biomod2, bossMaps, diffeR, dtwSat, EcoGenetics, esmisc, paleofire, prism, rsMove, and zonator.

[hadley]

@batpigandme hmmmm, the first case is because we've changed the structure of aes(): paste()ing it (which was never a good idea), no longer yields useful strings. It's hard to see what the problem is in the second case. Maybe we need another issue to track this? Would you mind creating?

@karawoo hmmmmm, I'll ask on twitter

[hadley]

@thomasp85 can you please take a look at BloodCancerMultiOmics2017? It fails with "plot.tag" is not a valid theme element name.

[hadley]

Phew, looks like we've done them all! Thanks for all your help! If you spotted an issue that affected multiple (i.e. >3 packages) can you please make sure it's tracked in an issue?

I think the next deadline is to try and get all issues fixed by Friday so we can kick off another round of revdeps this weekend.

[thomasp85]

Those that are not filled out in the sheet are they to be ignored?

[hadley]

Ooops, ignore that somehow I missed a whole bunch that haven't been analysed yet 😭

[thomasp85]

You raised my hope just to tear it down again...

[batpigandme]

If you spotted an issue that affected multiple (i.e. >3 packages) can you please make sure it's tracked in an issue?

Do you want me to open an issue for GGally-related probs?

[thomasp85]

ggmosaic fails due to changes made in the last version. Specifically it passes a list into x and y, which throws an error because x and y are checked for finite values during transformations.

I'm unsure whether why should even allow positional data to be lists..?

[thomasp85]

I'm clocking out and won't have as much time to go bug hunting in the coming days - feel free to tag me into issues that you think makes sense and I'll try to create the fixes, but I won't be filling in empty cells in the sheet for a couple of days

[batpigandme]

It appears that there are five cases of theme$panel.ontop) { : argument is of length zero.

[karawoo]

It appears that there are five cases of theme$panel.ontop) { : argument is of length zero.

A lot of packages have defined themes with complete = TRUE that are not actually complete, and since the last release we've gotten stricter about making sure that complete themes don't ever inherit defaults. I think these packages will need to fix their themes.

edit nvm #2606 should take care of it

[hadley]

I see quite a few Error in min(c(x, xmin), na.rm = TRUE) : invalid 'type' (list) of argument. @karawoo would you have time to look into? I think it's likely to be related to something we did to scales, and will probably need some digging in the other package source.

[karawoo]

I will try to look into it (maybe not today)

[hadley]

@karawoo I'll create an issue to track. edit: no I won't because you already have 😄

Unfortunately I'm teaching a workshop Thursday and Friday, so I'm trying to front-load as much work as possible. I'll try and review PRs during the breaks.

[hadley]

Ok, I think the majority of packages are now actually triaged, and we can now concentrate on diagnosing and fixing the issues under https://github.com/tidyverse/ggplot2/milestone/16

[hadley]

I'm going to close this issue because I think we've successfully identified all the revdep issues for this round. All going well we'll have another much smaller round on Monday, and I'll open a new issue for that.

[lock]

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/