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

CRAN check failing due to ggplot2 errors (only on Windows devel) #5702

Closed
samuel-marsh opened this issue Feb 22, 2024 · 5 comments
Closed

CRAN check failing due to ggplot2 errors (only on Windows devel) #5702

samuel-marsh opened this issue Feb 22, 2024 · 5 comments

Comments

@samuel-marsh
Copy link

Hi ggplot2 team,

I'm author and maintainer of CRAN package scCustomize. Starting yesterday (feb-21) I'm getting ggplot2 related errors when running devtools::check_win_devel() but did not receive same error when running on Friday Feb-16. The errors are coming from a function that I have never had issue with before and my debugging attempts have left me a little stumped.

The functions in question related to use of a custom ggplot2 theme. See here for full plotting code in function context plotting elements are as follows:

# "group.by" is user supplied variable/column name used in the created of data.frame "merged".
plot <- ggplot(data = merged, mapping = aes(x = .data[[group_by]], y = .data[["Median_nFeature_RNA"]], fill = .data[[group_by]])) +
      geom_boxplot(fill = "white") +
      geom_dotplot(binaxis ='y', stackdir = 'center', dotsize = dot_size) +
      scale_fill_manual(values = colors_use) +
      theme_ggprism_mod() +
      ggtitle(plot_title) +
      ylab(y_axis_label) +
      xlab("")

The error (that again was not present when running checks last week) is:

Error 1
* checking examples ... [25s] ERROR
Running examples in 'scCustomize-Ex.R' failed
The error most likely occurred in:

> base::assign(".ptime", proc.time(), pos = "CheckExEnv")
> ### Name: Plot_Median_Genes
> ### Title: Plot Median Genes per Cell per Sample
> ### Aliases: Plot_Median_Genes
> 
> ### ** Examples
> 
> library(Seurat)
> # Create example groups
> pbmc_small$sample_id <- sample(c("sample1", "sample2"), size = ncol(pbmc_small), replace = TRUE)
> 
> # Plot
> Plot_Median_Genes(seurat_object = pbmc_small, sample_col = "orig.ident",  group_by = "sample_id")
Bin width defaults to 1/30 of the range of the data. Pick better value with
`binwidth`.
Error in `plot_theme()`:
! The `legend.text.align` theme element is not defined in the element
  hierarchy.
Backtrace:1. ├─base (local) `<fn>`(x)
  2. └─ggplot2:::print.ggplot(x)
  3.   ├─ggplot2::ggplot_gtable(data)
  4.   └─ggplot2:::ggplot_gtable.ggplot_built(data)
  5.     └─ggplot2:::plot_theme(plot)
  6.       └─ggplot2:::validate_theme(theme)
  7.         └─base::mapply(...)
  8.           └─ggplot2 (local) `<fn>`(...)
  9.             └─cli::cli_abort(...)
 10.               └─rlang::abort(...)
Execution halted
* checking PDF version of manual ... [17s] OK
* checking HTML version of manual ... [21s] OK
* DONE
Status: 1 ERROR

The theme theme_ggprism_mod is extension of this theme from CRAN package ggprism (which does define legend.text.align)

My function extending this theme can be found here and the code is:

theme_ggprism_mod <- function(
  palette = "black_and_white",
  base_size = 14,
  base_family = "sans",
  base_fontface = "bold",
  base_line_size = base_size / 20,
  base_rect_size = base_size / 20,
  axis_text_angle = 0,
  border = FALSE
) {
  theme_prism(palette = palette,
              base_size = base_size,
              base_family = base_family,
              base_fontface = base_fontface,
              base_line_size = base_line_size,
              base_rect_size = base_rect_size,
              axis_text_angle = axis_text_angle,
              border = border) %+replace%
    theme(legend.title = element_text(hjust = 0),
          axis.text = element_text(size = rel(0.95), face = "plain")
    )
}

To debug things as this error does not occur on testing with any other platform I simply added dontrun to the examples and ran check again.

The identical error came back testing the example code of the theme extension in my package theme_ggprism_mod

library(ggplot2)
df <- data.frame(x = rnorm(n = 100, mean = 20, sd = 2), y = rbinom(n = 100, size = 100, prob = 0.2))
p <- ggplot(data = df, mapping = aes(x = x, y = y)) + geom_point(mapping = aes(color = 'red'))
p + theme_ggprism_mod()

To debug further I removed dontrun and swapped out my modified theme for the original theme from ggprism:

    plot <- ggplot(merged, aes(x = .data[["samples_plotting"]], y = .data[["Median_nFeature_RNA"]])) +
      geom_boxplot(fill = "white", outlier.color = NA) +
      geom_quasirandom() +
      ggtitle(plot_title) +
      ylab(y_axis_label) +
      xlab("") +
      theme_prism()

However, I still get the same error.

So at this point I'm a bit stumped because it seems my theme modification is fine and the plotting functions, theme modification, and ggprism have all been through multiple previous CRAN releases without issue.

Also as I mentioned these errors were not present in checks run on Feb 16th. Is this potentially just an issue of instability in the Windows R dev build? Since the errors were coming from ggplot2 that seemed unlikely to me but maybe you have more insight.

Hoping you might be able to provide some insight.
Thanks in advance!!
Sam

@teunbrand
Copy link
Collaborator

teunbrand commented Feb 22, 2024

Hi Sam,

Yesterday we submitted a new version of ggplot2 3.5.0 to CRAN, however, the submission is at the time of writing, not yet released. The scCustomize package has been on our radar for reverse dependency failures. When analysing these, we thought the problems originated from the ggprism package. We have submitted a fix to ggprism, see csdaw/ggprism#25, but this has not been merged and released at this point.

The issue at hand is that legend.text.align and legend.title.align have been deprecated. We have fallbacks for this deprecation in the theme() function, but ggprism appears to circumvent explicitly using theme() in the construction of their theme (from this line). As a result, these deprecated arguments cannot be processed, which is what the error indicates. This is really something that should be adressed by ggprism, not scCustomize, which is why we hadn't contacted you.

If you want to address this at the scCustomize side of things (which ideally shouldn't have to be the case), I'm going to recommend you to explicitly remove the legend.text.align and legend.title.align fields from the theme that you modify from ggprism. That should allow plots to be build again.

A reprex with the release candidate:

library(scCustomize)
#> Loading required package: Seurat
#> Loading required package: SeuratObject
#> Loading required package: sp
#> 
#> Attaching package: 'SeuratObject'
#> The following object is masked from 'package:base':
#> 
#>     intersect
#> scCustomize v2.0.1
#> If you find the scCustomize useful please cite.
#> See 'samuel-marsh.github.io/scCustomize/articles/FAQ.html' for citation info.
library(Seurat)

pbmc_small$sample_id <- sample(c("sample1", "sample2"), size = ncol(pbmc_small), replace = TRUE)

# Plot
p <- Plot_Median_Genes(
  seurat_object = pbmc_small, 
  sample_col = "orig.ident",  
  group_by = "sample_id"
)

# Does not work
p
#> Bin width defaults to 1/30 of the range of the data. Pick better value with
#> `binwidth`.
#> Error in `plot_theme()` at ggplot2/R/plot-build.R:178:3:
#> ! The `legend.text.align` theme element is not defined in the element
#>   hierarchy.

# At least renders without error. I'm not sure about the correctness of the plot.
p$theme[c("legend.text.align", "legend.title.align")] <- NULL
p
#> Bin width defaults to 1/30 of the range of the data. Pick better value with
#> `binwidth`.

Created on 2024-02-22 with reprex v2.1.0

If you want to test yourself and see if the solution works, you can install the release candidate as follows:

remotes::install_github("tidyverse/ggplot2", ref = remotes::github_pull("5592"))

I hope this answers most of your questions, and if you have more we'll be glad to hear them.

Best,
Teun

@samuel-marsh
Copy link
Author

Hi Tuen,

Thank you so much for quick reply and explanation!! That totally makes sense now given the deprecation and how the theme is created in ggprism.

I'm going to go ahead and make temp fix in scCustomize so that I can push my newest version to CRAN and I'll add quick ping on the ggprism PR and hopefully it can be addressed soon.

Thanks so much again!
Sam

@samuel-marsh
Copy link
Author

Hi @teunbrand,

So while waiting for fix from ggprism I decide to try just specifying ggplot2 version in DESCRIPTION to avoid 3.5.0 related errors.

When I use either

Imports: 
    circlize,
    cli (>= 3.2.0),
    cowplot,
    data.table,
    dplyr,
    forcats,
    ggbeeswarm,
    ggplot2 (< 3.5.0),
    ggprism,
    ggrastr,
    ...

or

Imports: 
    circlize,
    cli (>= 3.2.0),
    cowplot,
    data.table,
    dplyr,
    forcats,
    ggbeeswarm,
    ggplot2 (<= 3.4.4),
    ggprism,
    ggrastr,
    ...
* checking package dependencies ... ERROR
Package required and available but unsuitable version: 'ggplot2'

I double checked and it doesn't appear that any of the other packages that I specify version numbers for in my DESCRIPTION conflict with those in ggplot2 rc/3.5.0 branch so unsure what issue is.

If you have any thoughts or advice I would greatly appreciate it!

Thanks again!!
Sam

@teunbrand
Copy link
Collaborator

Hi Sam,

IIRC you can only specify a minimum version of a package, not a maximum.
Does removing the legend.text.align and legend.title.align items from the theme list in this definition not resolve all issues?

Best,
Teun

@samuel-marsh
Copy link
Author

Hi Teun,

Yes you're right! I was implementing wrong in my test code but that indeed works. Thanks so much again for the help and quick replies!!!!

Best,
Sam

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants