Skip to content
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

API seems broken after the latest updates in ggplot2 (>= 3.5.0) #195

Open
federicomarini opened this issue Feb 26, 2024 · 28 comments
Open
Labels
potential-bug Bug report filled by the user

Comments

@federicomarini
Copy link

federicomarini commented Feb 26, 2024

Describe the bug

After the latest changes introduced by ggplot2 3.5.0, the API for upset seems broken.

I noticed this behavior as this is triggered in the Bioconductor Build System for my package GeneTonic -> https://master.bioconductor.org/checkResults/3.19/bioc-LATEST/GeneTonic/nebbiolo1-checksrc.html

Code to reproduce

## taken from your vignette
library(ggplot2)
library(ComplexUpset)
movies = as.data.frame(ggplot2movies::movies)
head(movies, 3)
genres = colnames(movies)[18:24]
genres
movies[genres] = movies[genres] == 1
t(head(movies[genres], 3))
movies[movies$mpaa == '', 'mpaa'] = NA
movies = na.omit(movies)
upset(movies, genres, name='genre', width_ratio=0.1)
sessionInfo()

Expected behavior

The plot is not generated, and the screenshot below is generated instead.

> upset(movies, genres, name='genre', width_ratio=0.1)
Error in `plot_theme()`:
! The `legend.text.align` theme element is not defined in the element
  hierarchy.
Run `rlang::last_trace()` to see where the error occurred.

Screenshots

image

Context (required)

ComplexUpset version: 1.3.5

R version details
> R.Version()
$platform
[1] "x86_64-apple-darwin20"

$arch
[1] "x86_64"

$os
[1] "darwin20"

$system
[1] "x86_64, darwin20"

$status
[1] ""

$major
[1] "4"

$minor
[1] "3.0"

$year
[1] "2023"

$month
[1] "04"

$day
[1] "21"

$`svn rev`
[1] "84292"

$language
[1] "R"

$version.string
[1] "R version 4.3.0 (2023-04-21)"

$nickname
[1] "Already Tomorrow"

R session information
> sessionInfo()
R version 4.3.0 (2023-04-21)
Platform: x86_64-apple-darwin20 (64-bit)
Running under: macOS Monterey 12.7.1

Matrix products: default
BLAS:   /System/Library/Frameworks/Accelerate.framework/Versions/A/Frameworks/vecLib.framework/Versions/A/libBLAS.dylib 
LAPACK: /Library/Frameworks/R.framework/Versions/4.3-x86_64/Resources/lib/libRlapack.dylib;  LAPACK version 3.11.0

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

time zone: Europe/Berlin
tzcode source: internal

attached base packages:
[1] stats4    stats     graphics  grDevices utils     datasets  methods  
[8] base     

other attached packages:
 [1] ComplexUpset_1.3.5          ggplot2_3.5.0              
 [3] org.Hs.eg.db_3.18.0         AnnotationDbi_1.64.1       
 [5] DESeq2_1.42.0               SummarizedExperiment_1.32.0
 [7] Biobase_2.62.0              MatrixGenerics_1.14.0      
 [9] matrixStats_1.2.0           GenomicRanges_1.54.1       
[11] GenomeInfoDb_1.38.5         IRanges_2.36.0             
[13] S4Vectors_0.40.2            BiocGenerics_0.48.1        
[15] macrophage_1.18.0           GeneTonic_2.7.2            
[17] testthat_3.2.1             

loaded via a namespace (and not attached):
  [1] RColorBrewer_1.1-3      rstudioapi_0.15.0       jsonlite_1.8.8         
  [4] shape_1.4.6             magrittr_2.0.3          rmarkdown_2.25         
  [7] farver_2.1.1            GlobalOptions_0.1.2     fs_1.6.3               
 [10] zlibbioc_1.48.0         vctrs_0.6.5             memoise_2.0.1          
 [13] RCurl_1.98-1.14         htmltools_0.5.7         S4Arrays_1.2.0         
 [16] usethis_2.2.2           dynamicTreeCut_1.63-1   SparseArray_1.2.3      
 [19] sass_0.4.8              bslib_0.6.1             htmlwidgets_1.6.4      
 [22] desc_1.4.3              plotly_4.10.4           cachem_1.0.8           
 [25] igraph_2.0.1.1          mime_0.12               lifecycle_1.0.4        
 [28] iterators_1.0.14        pkgconfig_2.0.3         colourpicker_1.3.0     
 [31] Matrix_1.6-5            R6_2.5.1                fastmap_1.1.1          
 [34] GenomeInfoDbData_1.2.11 shiny_1.8.0             clue_0.3-65            
 [37] digest_0.6.34           colorspace_2.1-0        shinycssloaders_1.0.0  
 [40] patchwork_1.2.0         rprojroot_2.0.4         pkgload_1.3.4          
 [43] RSQLite_2.3.5           labeling_0.4.3          fansi_1.0.6            
 [46] httr_1.4.7              polyclip_1.10-6         abind_1.4-5            
 [49] compiler_4.3.0          remotes_2.4.2.1         bit64_4.0.5            
 [52] withr_3.0.0             doParallel_1.0.17       BiocParallel_1.36.0    
 [55] viridis_0.6.5           DBI_1.2.1               shinyAce_0.4.2         
 [58] dendextend_1.17.1       pkgbuild_1.4.3          ggforce_0.4.1          
 [61] MASS_7.3-60.0.1         DelayedArray_0.28.0     sessioninfo_1.2.2      
 [64] rjson_0.2.21            tools_4.3.0             httpuv_1.6.14          
 [67] tippy_0.1.0             glue_1.7.0              promises_1.2.1         
 [70] grid_4.3.0              bs4Dash_2.3.3           cluster_2.1.6          
 [73] generics_0.1.3          gtable_0.3.4            tidyr_1.3.1            
 [76] data.table_1.15.0       utf8_1.2.4              XVector_0.42.0         
 [79] ggrepel_0.9.5           foreach_1.5.2           pillar_1.9.0           
 [82] stringr_1.5.1           later_1.3.2             rintrojs_0.3.4         
 [85] circlize_0.4.15         dplyr_1.1.4             tweenr_2.0.2           
 [88] lattice_0.22-5          bit_4.0.5               tidyselect_1.2.0       
 [91] GO.db_3.18.0            ComplexHeatmap_2.18.0   locfit_1.5-9.8         
 [94] Biostrings_2.70.2       miniUI_0.1.1.1          knitr_1.45             
 [97] gridExtra_2.3           ggplot2movies_0.0.1     xfun_0.41              
[100] expm_0.999-9            brio_1.1.4              devtools_2.4.5         
[103] DT_0.31                 visNetwork_2.1.2        stringi_1.8.3          
[106] lazyeval_0.2.2          shinyWidgets_0.8.1      evaluate_0.23          
[109] codetools_0.2-19        tibble_3.2.1            cli_3.6.2              
[112] xtable_1.8-4            jquerylib_0.1.4         munsell_0.5.0          
[115] Rcpp_1.0.12             png_0.1-8               parallel_4.3.0         
[118] ellipsis_0.3.2          blob_1.2.4              profvis_0.3.8          
[121] urlchecker_1.0.1        bitops_1.0-7            viridisLite_0.4.2      
[124] scales_1.3.0            backbone_2.1.3          ggridges_0.5.6         
[127] purrr_1.0.2             crayon_1.5.2            GetoptLong_1.0.5       
[130] rlang_1.1.3             KEGGREST_1.42.0  
@federicomarini federicomarini added the potential-bug Bug report filled by the user label Feb 26, 2024
@federicomarini
Copy link
Author

Thanks in advance for looking into this!

@smped
Copy link

smped commented Feb 27, 2024

Thanks for flagging this so quickly @federicomarini . Can also confirm that two of my packages which depend on ComplexUpset are also failing to build on Bioconductor with the same error, i.e.

The `legend.text.align` theme element is not defined in the element hierarchy

https://master.bioconductor.org/checkResults/3.18/bioc-LATEST/extraChIPs/nebbiolo2-buildsrc.html

@smped
Copy link

smped commented Feb 27, 2024

Ok, I think this is a curly one. I ran the following code, noting that upset_default_themes() simply calls theme_minimal(). Testing this showed that the call to theme_minimal() performed by upset_default_themes() returned a version of theme_minimal() from ggplot2 v3.4.x, not 3.5.0, even though I have ggplot2 v3.5.0 installed.

setdiff(names(ComplexUpset::upset_default_themes()$default[[1]]), names(formals(theme)))

[1] "legend.text.align" "legend.title.align"

I wondered if this meant ComplexUpset had been installed from a binary so I ran remove.packages("ComplexUpset") and reinstalled using install.packages("ComplexUpset", type = "source"). Now the conflict has resolved so I think we need to remove any cached packages from github actions and I'll also check to see if ComplexUpset can be re-installed on the Bioconductor Build System.

@federicomarini
Copy link
Author

Well done in investigating this a bit.
Potentially, the GHA system should be able to "catch these things" in case newer package versions come along.
And the Bioc BBS is actually always picking the latest versions. I'll keep this monitored in the coming days, as we are somehow approaching high noon (= the upcoming release deadlines).

@smped
Copy link

smped commented Feb 27, 2024

Thanks. I can't see the BBS picking this up as there's no new version of ComplexUpset to trigger a reinstallation of ComplexUpset. The currently installed version will continue to return the incompatible values from it's call to theme_minimal() until it's reinstalled on a system after ggplot2 v3.5.0 has been installed

@lisiarend
Copy link

Ok, I think this is a curly one. I ran the following code, noting that upset_default_themes() simply calls theme_minimal(). Testing this showed that the call to theme_minimal() performed by upset_default_themes() returned a version of theme_minimal() from ggplot2 v3.4.x, not 3.5.0, even though I have ggplot2 v3.5.0 installed.

setdiff(names(ComplexUpset::upset_default_themes()$default[[1]]), names(formals(theme)))

[1] "legend.text.align" "legend.title.align"

I wondered if this meant ComplexUpset had been installed from a binary so I ran remove.packages("ComplexUpset") and reinstalled using install.packages("ComplexUpset", type = "source"). Now the conflict has resolved so I think we need to remove any cached packages from github actions and I'll also check to see if ComplexUpset can be re-installed on the Bioconductor Build System.

I could not get it fixed with your solution.

@lshep
Copy link

lshep commented Feb 27, 2024

The Bioconductor Build System would like to avoid having to uninstall and reinstall a package manually since this hides what a user sees on their local system. A user with the previously installed would also have to be aware to uninstall and reinstall manually. Ideally ComplexUpset would bump their version forcing a new version install for all users universally if this is indeed all that needed to be done.

@federicomarini
Copy link
Author

if this is indeed all that needed to be done

And if this is all, well, I think we can already prep up a "close to fake" PR to have the new version picked up by the related systems 😅

@federicomarini
Copy link
Author

Ok, I think this is a curly one. I ran the following code, noting that upset_default_themes() simply calls theme_minimal(). Testing this showed that the call to theme_minimal() performed by upset_default_themes() returned a version of theme_minimal() from ggplot2 v3.4.x, not 3.5.0, even though I have ggplot2 v3.5.0 installed.

setdiff(names(ComplexUpset::upset_default_themes()$default[[1]]), names(formals(theme)))

[1] "legend.text.align" "legend.title.align"
I wondered if this meant ComplexUpset had been installed from a binary so I ran remove.packages("ComplexUpset") and reinstalled using install.packages("ComplexUpset", type = "source"). Now the conflict has resolved so I think we need to remove any cached packages from github actions and I'll also check to see if ComplexUpset can be re-installed on the Bioconductor Build System.

I could not get it fixed with your solution.

FWIW, the suggested solution from @smped worked in my hands.

Uninstalling explicitly, installing (from source, dunno if that really helps, but hey, it worked), re-running the example and the functions from my package using that: ✅

@smped
Copy link

smped commented Feb 27, 2024

Thanks @lshep & @federicomarini . I've added a bug catcher to my function so I think I'm OK with this one, but it did seem a bit odd. I'm passing arguments through the dots which I have as dotArgs

valid_theme_args <- names(formals(theme))
  dotArgs$themes <- lapply(
      dotArgs$themes,
      \(x) lapply(x, \(y) do.call("theme", y[names(y) %in% valid_theme_args]))
  )

Sorry to hear your attempt didn't work either @lisiarend. Hopefully the package authors will be able to help.

@jctourtellotte
Copy link

jctourtellotte commented Mar 4, 2024

So.... will uninstalling and reinstalling fix this? I am going to try that, because I am also having this issue and I'm mid-dissertation with the use of an UpSet plot being fairly significant in one chunk of the analysis I'm writing up. Is there a nail biting emoji?

Edit: uninstalling and reinstalling from source (install.packages("ComplexUpset", type = "source")) worked. The from source was key, as I had tried to uninstall/reinstall through the RStudio GUI and that didn't work.

@federicomarini
Copy link
Author

Glad it worked @jctourtellotte !
The issue still persists for (build) systems where this should likely be less triggered by hand, but rather picked up automatically. My guess is that a simple version bump could do it.

If that did not solve it, you could ideally revert back to the previous ggplot2 version. And/or you can protect yourself from such things with anything on the line of renv to keep package collection snapshots.

@adrientaudiere
Copy link

Hi all, my R package depends on ComplexUpset package and I have an error from CRAN which seems very similar to the problem you are talking about in this issue (link to CRAN logs. I put an extract of the error below. CRAN team asked me to resolve this error. How can I fix in this special case?

[...]
Error(s) in re-building vignettes:
--- re-building ‘MiscMetabar.Rmd’ using rmarkdown

Quitting from lines 68-69 [unnamed-chunk-4] (MiscMetabar.Rmd)
Error: processing vignette 'MiscMetabar.Rmd' failed with diagnostics:
The `legend.text.align` theme element is not defined in the element
hierarchy.
--- failed re-building ‘MiscMetabar.Rmd’
[...]

@federicomarini
Copy link
Author

Hm, not sure you can solve this, "alone".
Since you do depend on this package, there's not too much you can do.

Maybe you can prevent your package from being archived on short term is to point towards this discussion, in the hope they understand it really does not relate to a mishap on your end @adrientaudiere ?

@krassowski
Copy link
Owner

So what specifically should this package do?

@federicomarini
Copy link
Author

You mean ComplexUpset?

I think the solution proposal that @smped was pointing to is that you Michal would simply issue a new release with a version bump.
I know it does not sound too complex, but it seemed to have done the job.
I guess the fails on the CRAN and Bioc Build systems is due to the fact of

  • the systems picking the new ggplot2 version, and using that
  • no new version for ComplexUpset being picked, so no need to reinstall
  • ... leading to the disagreement in the elements of the theme

I also could not find any better solution so far, but it is worth a try from my point of view.

@smped
Copy link

smped commented Mar 7, 2024

Hi @krassowski ,

I think the real issue has been caused by a quirk in the R ecosystem & the decision within the tidyverse developer community to not enforce backwards compatibility.

The simplest fix to me is to push out a version bump which enforces a dependency on ggplot2 v3.5.0, and that should resolve this specific error, although who knows what may be coming in the future. So just a simple change the DESCRIPTION file & everything should be fine from our end.

The catch may be in the tests. I forked & tried the above in order to make a pull request, but all tests in test-other-visual.R failed for me & I didn't have time to figure them all out. And there's the ggplot2 deprecation of aes_ and aes_string() to contend with. (...sigh...). FWIW, I think I lose a week or two every year trying to fix my packages after the tidyverse breaks a bunch of things. Brilliant packages for users, terrible for developers.

@adrientaudiere
Copy link

Thanks @smped and @federicomarini for your comprehensive answer. Maybe I can enforce a dependency on ggplot2 v3.5.0 in my own package DESCRIPTION file. @krassowski do you plan to make a new release enforcing ggplot2 v3.5.0 soon?

@krassowski
Copy link
Owner

Yes, I can do that over the weekend.

@federicomarini
Copy link
Author

Very appreciated, Mike, thanks!

@smped
Copy link

smped commented Mar 7, 2024

Brilliant. Thanks @krassowski . Much appreciated indeed

@adrientaudiere
Copy link

Oh yes, Thanks a lot @krassowski!

@krassowski
Copy link
Owner

Hi folks, I had submitted ComplexUpset 1.3.6 to CRAN. Fingers crossed.

@krassowski
Copy link
Owner

It appears that the submission to CRAN was rejected via an automated email because CRAN's link checker thinks that "https://anaconda.org/conda-forge/r-complexupset" does not work (but it does - you can check it yourself):

Dear maintainer,

package ComplexUpset_1.3.6.tar.gz does not pass the incoming checks automatically, please see the following pre-tests:
Windows: https://win-builder.r-project.org/incoming_pretest/ComplexUpset_1.3.6_20240309_162121/Windows/00check.log
Status: 1 NOTE
Debian: https://win-builder.r-project.org/incoming_pretest/ComplexUpset_1.3.6_20240309_162121/Debian/00check.log
Status: 1 NOTE

Last released version's CRAN status: OK: 12
See: https://cran.r-project.org/web/checks/check_results_ComplexUpset.html

CRAN Web: https://cran.r-project.org/package=ComplexUpset

Please fix all problems and resubmit a fixed version via the webform.

It is not an error, not even a warning. It is just a note...

The 00check.log contents:

* using log directory 'd:/RCompile/CRANincoming/R-devel/ComplexUpset.Rcheck'
* using R Under development (unstable) (2024-03-08 r86072 ucrt)
* using platform: x86_64-w64-mingw32
* R was compiled by
    gcc.exe (GCC) 12.3.0
    GNU Fortran (GCC) 12.3.0
* running under: Windows Server 2022 x64 (build 20348)
* using session charset: UTF-8
* checking for file 'ComplexUpset/DESCRIPTION' ... OK
* checking extension type ... Package
* this is package 'ComplexUpset' version '1.3.6'
* package encoding: UTF-8
* checking CRAN incoming feasibility ... [19s] NOTE
Maintainer: 'Michał Krassowski <[email protected]>'

Package CITATION file contains call(s) to old-style citEntry().  Please
use bibentry() instead.

Found the following (possibly) invalid URLs:
  URL: https://anaconda.org/conda-forge/r-complexupset
    From: README.md
    Status: 400
    Message: Bad Request

Size of tarball: 5107480 bytes
* checking package namespace information ... OK
* checking package dependencies ... OK
* checking if this is a source package ... OK
* checking if there is a namespace ... OK
* checking for hidden files and directories ... OK
* checking for portable file names ... OK
* checking serialization versions ... OK
* checking whether package 'ComplexUpset' can be installed ... OK
* checking installed package size ... OK
* checking package directory ... OK
* checking for future file timestamps ... OK
* checking 'build' directory ... OK
* checking DESCRIPTION meta-information ... OK
* checking top-level files ... OK
* checking for left-over files ... OK
* checking index information ... OK
* checking package subdirectories ... OK
* checking R files for non-ASCII characters ... OK
* checking R files for syntax errors ... OK
* checking whether the package can be loaded ... OK
* checking whether the package can be loaded with stated dependencies ... OK
* checking whether the package can be unloaded cleanly ... OK
* checking whether the namespace can be loaded with stated dependencies ... OK
* checking whether the namespace can be unloaded cleanly ... OK
* checking loading without being on the library search path ... OK
* checking use of S3 registration ... OK
* checking dependencies in R code ... OK
* checking S3 generic/method consistency ... OK
* checking replacement functions ... OK
* checking foreign function calls ... OK
* checking R code for possible problems ... OK
* checking Rd files ... OK
* checking Rd metadata ... OK
* checking Rd line widths ... OK
* checking Rd cross-references ... OK
* checking for missing documentation entries ... OK
* checking for code/documentation mismatches ... OK
* checking Rd \usage sections ... OK
* checking Rd contents ... OK
* checking for unstated dependencies in examples ... OK
* checking installed files from 'inst/doc' ... OK
* checking files in 'vignettes' ... OK
* checking examples ... OK
* checking for unstated dependencies in vignettes ... OK
* checking package vignettes ... OK
* checking re-building of vignette outputs ... OK
* checking PDF version of manual ... [13s] OK
* checking HTML version of manual ... OK
* DONE
Status: 1 NOTE

@smped
Copy link

smped commented Mar 11, 2024

Gosh. That seems really weird to get a fail based an a URL in a README!

Thanks for working in this over your weekend too & hopefully there's an easy fix.

@federicomarini
Copy link
Author

I know it is annoying @krassowski , thanks for taking care of this so far.

Maybe you can argue that with one email back&forth? I recall once I had an issue (similar, not same) because they did not want me to leave a URL that was redirecting to our institute, but only accepted the direct link.
But this does not seem to be the case here...

The bibentry() thing is something you could easily solve?

@adrientaudiere
Copy link

adrientaudiere commented Mar 11, 2024

Thanks @krassowski. I was able to resubmit my package by forcing the ggplot2 version in the DESCRIPTION file of my package MiscMetabar.

Concerning the url, sometimes, one site may be dead for a little time and that may arise when CRAN is checking your package. This has already happened to me, but for a link to a much less stable site than anaconda.org.

Its is strange, especially as the DESCRIPTION file has not changed. I think if you solve the bibentry() note you can resubmit to CRAN adding in the field Optional comment: that you are surprised by the note about the url.

adrientaudiere added a commit to adrientaudiere/MiscMetabar that referenced this issue Apr 17, 2024
@smped
Copy link

smped commented Apr 20, 2024

Hi again @krassowski,

Just following up on this given the pending Bioconductor release. Have you managed to make any progress with CRAN?

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
potential-bug Bug report filled by the user
Projects
None yet
Development

No branches or pull requests

7 participants