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

thematic_on breaks if another package includes ggplot (maybe?) #90

Closed
elfatherbrown opened this issue Feb 16, 2021 · 16 comments · Fixed by #91
Closed

thematic_on breaks if another package includes ggplot (maybe?) #90

elfatherbrown opened this issue Feb 16, 2021 · 16 comments · Fixed by #91

Comments

@elfatherbrown
Copy link

elfatherbrown commented Feb 16, 2021

The problem

I have a shiny app built as a package with golem that uses many plotting libraries including ggplot2 and as soon as I tried thematic_on or thematic_shiny I got:

"Error in get(S3[i, 1L], mode = "function", envir = parent.frame()): invalid first argument".

I digged in and discovered that the same problem that happens with my package, happens with others that include ggplot2:

    library(thematic)
#> This version of thematic is designed to work with rmarkdown version 2.7.0 or higher. Consider upgrading via remotes::install_github('rstudio/rmarkdown#1706')
thematic::thematic_on()
thematic::thematic_off()
library(survMisc)
#> Loading required package: survival
thematic::thematic_on()
#> Error in get(S3[i, 1L], mode = "function", envir = parent.frame()): invalid first argument

Created on 2021-02-16 by the reprex package (v1.0.0)

Further digging on thematic's code

On another box I set out to debug thematic. I cloned it and this is what I did.

On this function I put a browser:

ggplot_build_set <- function() {
  browser()
  if (!is_installed("ggplot2")) return(NULL)
  ggplot_build_restore()
  # Note that assignInNamespace() does S3 method registration, but to
  # find the relevant generic, it looks in the parent.frame()...
  # so this line here is prevent that from failing if ggplot2 hasn't been attached
  # https://github.com/wch/r-source/blob/d0ede8/src/library/utils/R/objects.R#L472
  ggplot_build <- getFromNamespace("ggplot_build", "ggplot2")
  .globals$ggplot_build <- getFromNamespace("ggplot_build.ggplot", "ggplot2")
  assign_in_namespace <- assignInNamespace
  assign_in_namespace("ggplot_build.ggplot", ggthematic_build, "ggplot2")
}

Discovered that the aforementioned "get" problem with S3 comes from assign_in_namespace("ggplot_build.ggplot", ggthematic_build, "ggplot2"). So I copied the assign_in_namespace (from assignInNamespace, as it does in the code), but set a browser on that.

What I discovered is that inside the assignInNamespace code, down by the line:

...
genfun <- get(S3[i, 1L], mode = "function", envir = parent.frame())
...

S3[i,1L] returns a list that contains the name of the function, but get wants a string. So, as you can see:

image

This goes away if one rewrites with:

...
genfun <- get(S3[i, 1L][[1]], mode = "function", envir = parent.frame())
...

Or at least the error does. Im not keen on hacking away at this because im pretty much a newb (i've no idea how S3 or namespaces actually work), but I think this is either a freakish mismanagement of my base packages (e.g. i carry a bug in utils::assignInNamespace), or a particular use of it that got mangled somewhere.

I LOVE the thematic idea. I want to use it! Let me know if I can help out.

@elfatherbrown elfatherbrown changed the title thematic_on breaks if another package includes ggplot thematic_on breaks if another package includes ggplot (maybe?) Feb 16, 2021
@elfatherbrown

This comment has been minimized.

@cpsievert
Copy link
Collaborator

Interesting, thanks for the report. Is your package publicly available?

@cpsievert

This comment has been minimized.

@cpsievert

This comment has been minimized.

@elfatherbrown

This comment has been minimized.

@cpsievert

This comment has been minimized.

@elfatherbrown

This comment has been minimized.

@elfatherbrown

This comment has been minimized.

@cpsievert
Copy link
Collaborator

cpsievert commented Feb 16, 2021

Looks like the problem derives from the zoo package (a dependency of these packages):

> library(zoo); thematic::thematic_on()
Error in get(S3[i, 1L], mode = "function", envir = parent.frame()) : 
  invalid first argument

@cpsievert
Copy link
Collaborator

cpsievert commented Feb 17, 2021

Turns out that library(zoo) causes this problem since it's calling registerS3method() in an .onLoad() hook, which unfortunately causes assignInNamespace() to no longer work (this seems like a bug in R itself). A minimal example:

library(ggplot2)
# works
ggplotBuild <- getFromNamespace("ggplot_build.ggplot", "ggplot2")
assignInNamespace("ggplot_build.ggplot", function(x) { warning("owned"); ggplotBuild(x) }, "ggplot2")
qplot(1:10)

# doesn't work
library(zoo)
assignInNamespace("ggplot_build.ggplot", function(x) { warning("owned"); ggplotBuild(x) }, "ggplot2")
#> Error in get(S3[i, 1L], mode = "function", envir = parent.frame()) : 
#>   invalid first argument

And this happens because the S3 lookup matrix turns into a list when registerS3method() happens:

is.matrix(.getNamespaceInfo(asNamespace("ggplot2"), "S3methods"))
#> TRUE
library(zoo)
is.matrix(.getNamespaceInfo(asNamespace("ggplot2"), "S3methods"))
#> FALSE

I may submit a bug to R core for this, but in the meantime, I think I can have thematic workaround this

@elfatherbrown
Copy link
Author

Tested s3methods-matrix branch. My problem has gone away. Thanks!

cpsievert added a commit that referenced this issue Feb 19, 2021
… .onLoad() to register ggplot2 methods (#91)

* Close #90: Get thematic working with packages that use registerS3method() inside .onLoad() to register ggplot2 methods

* Improve comment and also ensure S3methods matrix when restoring previous ggplot_build

* New ragg baselines

* update news; approve ragg mac baseline
@ghost
Copy link

ghost commented Mar 7, 2021

Turns out that library(zoo) causes this problem since it's calling registerS3method() in an .onLoad() hook, which unfortunately causes assignInNamespace() to no longer work (this seems like a bug in R itself). A minimal example:

library(ggplot2)
# works
ggplotBuild <- getFromNamespace("ggplot_build.ggplot", "ggplot2")
assignInNamespace("ggplot_build.ggplot", function(x) { warning("owned"); ggplotBuild(x) }, "ggplot2")
qplot(1:10)

# doesn't work
library(zoo)
assignInNamespace("ggplot_build.ggplot", function(x) { warning("owned"); ggplotBuild(x) }, "ggplot2")
#> Error in get(S3[i, 1L], mode = "function", envir = parent.frame()) : 
#>   invalid first argument

And this happens because the S3 lookup matrix turns into a list when registerS3method() happens:

is.matrix(.getNamespaceInfo(asNamespace("ggplot2"), "S3methods"))
#> TRUE
library(zoo)
is.matrix(.getNamespaceInfo(asNamespace("ggplot2"), "S3methods"))
#> FALSE

I may submit a bug to R core for this, but in the meantime, I think I can have thematic workaround this

Hi. I still face the same problem

#> Error in get(S3[i, 1L], mode = "function", envir = parent.frame()) : 
#>   invalid first argument

Here is my sessionInfo

R version 4.0.4 (2021-02-15)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 7 x64 (build 7601) Service Pack 1

Matrix products: default

locale:
[1] LC_COLLATE=Greek_Greece.1253 
[2] LC_CTYPE=Greek_Greece.1253   
[3] LC_MONETARY=Greek_Greece.1253
[4] LC_NUMERIC=C                 
[5] LC_TIME=Greek_Greece.1253    

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

other attached packages:
[1] zoo_1.8-8     ggplot2_3.3.3

loaded via a namespace (and not attached):
 [1] Rcpp_1.0.6        bslib_0.2.4       compiler_4.0.4   
 [4] pillar_1.5.1      jquerylib_0.1.3   tools_4.0.4      
 [7] odbc_1.3.0        digest_0.6.27     bit_4.0.4        
[10] lattice_0.20-41   gtable_0.3.0      evaluate_0.14    
[13] jsonlite_1.7.2    lifecycle_1.0.0   tibble_3.1.0     
[16] debugme_1.1.0     pkgconfig_2.0.3   rlang_0.4.10     
[19] DBI_1.1.1         rstudioapi_0.13   yaml_2.2.1       
[22] xfun_0.21         withr_2.4.1       dplyr_1.0.5      
[25] knitr_1.31        htmlwidgets_1.5.3 generics_0.1.0   
[28] vctrs_0.3.6       sass_0.3.1        hms_1.0.0        
[31] thematic_0.1.1    bit64_4.0.5       grid_4.0.4       
[34] tidyselect_1.1.0  glue_1.4.2        R6_2.5.0         
[37] fansi_0.4.2       rmarkdown_2.7     librarian_1.7.0  
[40] farver_2.1.0      purrr_0.3.4       blob_1.2.1       
[43] magrittr_2.0.1    scales_1.1.1      ellipsis_0.3.1   
[46] htmltools_0.5.1.1 rsconnect_0.8.16  assertthat_0.2.1 
[49] colorspace_2.0-0  labeling_0.4.2    utf8_1.1.4       
[52] munsell_0.5.0     crayon_1.4.1     

@cpsievert
Copy link
Collaborator

You need the development version remotes::install_github('rstudio/thematic')

@michaelgaunt404
Copy link

michaelgaunt404 commented Jun 18, 2021

Hi, I'm still receiving this error as well.
This is my yaml and first code chunk:

---
title: "ETAN Database Exploration Tool"
output:
  html_document:
    theme:
      bg: "#002b36"
      fg: "#eee8d5"
      primary: "#2aa198"
      font: "auto"

# runtime: shiny
---

'''{r setup, include=FALSE}
knitr::opts_chunk$set(
  cache = FALSE, cache.lazy = FALSE, autodep = TRUE, warning = FALSE, 
  message = FALSE, echo = TRUE, dpi = 180,
  fig.width = 8, fig.height = 5, echo = FALSE
  )
thematic::thematic_rmd(font = "auto")

'''

I'm receiving the following error both in isolated code chunks that generate ggplot visuals and during markdown knit.

> > sessionInfo()
R version 3.6.2 (2019-12-12)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 17134)

Matrix products: default

locale:
[1] LC_COLLATE=English_United States.1252  LC_CTYPE=English_United States.1252   
[3] LC_MONETARY=English_United States.1252 LC_NUMERIC=C                          
[5] LC_TIME=English_United States.1252    

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

other attached packages:
 [1] shiny_1.6.0          gridExtra_2.3        scales_1.1.1         ggraph_2.0.5        
 [5] tidygraph_1.2.0      networkD3_0.4        thematic_0.1.2.1     ggridges_0.5.3      
 [9] here_1.0.1           data.validator_0.1.5 skimr_2.1.2          DT_0.17             
[13] lubridate_1.7.9.2    data.table_1.13.2    janitor_2.1.0        readxl_1.3.1        
[17] magrittr_2.0.1       forcats_0.5.1        stringr_1.4.0        dplyr_1.0.4         
[21] purrr_0.3.4          readr_1.4.0          tidyr_1.1.3          tibble_3.1.0        
[25] ggplot2_3.3.3        tidyverse_1.3.0     

loaded via a namespace (and not attached):
 [1] colorspace_2.0-0     ellipsis_0.3.1       rsconnect_0.8.16     rprojroot_2.0.2     
 [5] snakecase_0.11.0     base64enc_0.1-3      fs_1.5.0             rstudioapi_0.13     
 [9] farver_2.0.3         rstan_2.21.2         graphlayouts_0.7.1   ggrepel_0.9.1       
[13] fansi_0.4.2          xml2_1.3.2           codetools_0.2-16     knitr_1.31          
[17] polyclip_1.10-0      jsonlite_1.7.2       packrat_0.5.0        broom_0.7.5         
[21] gganimate_1.0.7      dbplyr_2.1.0         ggforce_0.3.2        compiler_3.6.2      
[25] httr_1.4.2           backports_1.2.1      assertthat_0.2.1     fastmap_1.0.1       
[29] cli_2.3.1            later_1.1.0.1        tweenr_1.0.1         htmltools_0.5.1.1   
[33] prettyunits_1.1.1    tools_3.6.2          igraph_1.2.6         gtable_0.3.0        
[37] glue_1.4.2           V8_3.4.0             Rcpp_1.0.6           jquerylib_0.1.3     
[41] cellranger_1.1.0     vctrs_0.3.6          crosstalk_1.1.1      xfun_0.19           
[45] ps_1.3.2             rvest_0.3.6          mime_0.9             lifecycle_1.0.0     
[49] MASS_7.3-51.4        hms_1.0.0            promises_1.1.1       parallel_3.6.2      
[53] inline_0.3.17        yaml_2.2.1           curl_4.3             sass_0.3.1          
[57] loo_2.4.1            StanHeaders_2.21.0-6 stringi_1.5.3        gifski_0.8.6        
[61] pkgbuild_1.2.0       repr_1.1.3           matrixStats_0.57.0   rlang_0.4.10        
[65] pkgconfig_2.0.3      evaluate_0.14        lattice_0.20-38      htmlwidgets_1.5.3   
[69] labeling_0.4.2       tidyselect_1.1.0     processx_3.4.4       plyr_1.8.6          
[73] R6_2.5.0             generics_0.1.0       DBI_1.1.1            pillar_1.6.0        
[77] haven_2.3.1          withr_2.4.1          modelr_0.1.8         crayon_1.4.1        
[81] utf8_1.1.4           rmarkdown_2.7        viridis_0.5.1        progress_1.2.2      
[85] grid_3.6.2           callr_3.5.1          reprex_1.0.0         digest_0.6.27       
[89] xtable_1.8-4         httpuv_1.5.4         RcppParallel_5.0.2   stats4_3.6.2        
[93] munsell_0.5.0        viridisLite_0.3.0    bslib_0.2.4 

@cpsievert
Copy link
Collaborator

cpsievert commented Jun 23, 2021

@michaelgaunt404 your theme definition is off. You probably want base_font (not font) set to a Google Font? https://rstudio.github.io/bslib/articles/bslib.html#custom-themes

@cpsievert
Copy link
Collaborator

cpsievert commented Jun 23, 2021

If that doesn't fix your issues, you might need to update to a more recent version of R

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