lawremi / ggbio

Grid and ggplot2 based visualization for biological data
111 stars 24 forks source link

Problems with `geom_bar()` when ggbio is loaded #161

Closed grimbough closed 2 years ago

grimbough commented 2 years ago

I'm experiencing an issue where the ggbio::geom_bar() method isn't playing nicely with some downstream package and is failing to produce plots when given a simple data.frame.

I can fix the problem by explicitly using ggplot::geom_bar(), which I why I suspect something in ggbio.

This only occurs with Bioconductor 3.15 & R 4.2 and wasn't the behaviour with previous versions. I've included a minimal example demonstrating the issue. Is this the expected behaviour going forward, and I should modify my code to specify ggplot::geom_bar() ?

Working with BioC 3.14 / R 4.1

suppressPackageStartupMessages(library(ggbio))

data <- data.frame(
  name=c("A","B","C","D","E") ,  
  value=c(3,12,5,18,45)
)

ggplot(data, aes(x=name, y=value)) + 
  geom_bar(stat = "identity")

sessionInfo()
#> R version 4.1.3 (2022-03-10)
#> Platform: x86_64-pc-linux-gnu (64-bit)
#> Running under: Ubuntu 20.04.4 LTS
#> 
#> Matrix products: default
#> BLAS/LAPACK: /usr/lib/x86_64-linux-gnu/openblas-pthread/libopenblasp-r0.3.8.so
#> 
#> locale:
#>  [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C              
#>  [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8    
#>  [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8   
#>  [7] LC_PAPER=en_US.UTF-8       LC_NAME=C                 
#>  [9] LC_ADDRESS=C               LC_TELEPHONE=C            
#> [11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       
#> 
#> attached base packages:
#> [1] stats     graphics  grDevices utils     datasets  methods   base     
#> 
#> other attached packages:
#> [1] ggbio_1.42.0        ggplot2_3.3.5       BiocGenerics_0.40.0
#> ...

Failing with BioC 3.15 / R 4.2

suppressPackageStartupMessages(library(ggbio))

data <- data.frame(
     name=c("A","B","C","D","E") ,  
     value=c(3,12,5,18,45)
)

ggplot(data, aes(x=name, y=value)) + 
       geom_bar(stat = "identity")
#> Error in `ggplot_add()`:
#> ! Can't add `e2` to a ggplot object.

sessionInfo()
#> R Under development (unstable) (2022-03-14 r81896)
#> Platform: x86_64-pc-linux-gnu (64-bit)
#> Running under: Ubuntu 20.04.4 LTS
#> 
#> Matrix products: default
#> BLAS/LAPACK: /usr/lib/x86_64-linux-gnu/openblas-pthread/libopenblasp-r0.3.8.so
#> 
#> locale:
#>  [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C              
#>  [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8    
#>  [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8   
#>  [7] LC_PAPER=en_US.UTF-8       LC_NAME=C                 
#>  [9] LC_ADDRESS=C               LC_TELEPHONE=C            
#> [11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       
#> 
#> attached base packages:
#> [1] stats     graphics  grDevices utils     datasets  methods   base     
#> 
#> other attached packages:
#> [1] ggbio_1.43.0        ggplot2_3.3.5       BiocGenerics_0.41.2
#> ... 
lawremi commented 2 years ago

Thoughts? @sanchit-saini

sanchit-saini commented 2 years ago

It is a bug that got introduced in this commit bfcf1b11c2.

https://github.com/lawremi/ggbio/blob/7f5f384f80ae5cfefb2839946e55d18d915775b5/R/hack.R#L110-L125

method0 should have been called with do.call to properly pass the list of arguments.

 tm <- try({res <- do.call(method0, args)}, silent = TRUE)
grimbough commented 2 years ago

Thanks for sorting this. Did the patch make it into the version on Bioconductor? I don't see it in https://code.bioconductor.org/browse/ggbio/commits/RELEASE_3_15 and I'm still experiencing the error.

lawremi commented 2 years ago

I just pushed a new ggbio version to release with this fix.

grimbough commented 2 years ago

I just pushed a new ggbio version to release with this fix.

Awesome, thanks a lot.