Bioconductor / BiocStyle

Issues and pull requests for BiocStyle should go here.
12 stars 19 forks source link

Rmd files fail to render with static svg? #71

Open mtmorgan opened 4 years ago

mtmorgan commented 4 years ago

The RaggedExperiment build report started to error across all platforms, maybe because of a change in magick? A reproducible example is a file test.Rmd containing

---
title: "test"
vignette: |
  %\VignetteEngine{knitr::rmarkdown}
output:
  BiocStyle::html_document
---
```{r}
knitr::include_graphics("RaggedExperiment.svg")
```

with RaggedExperiment.svg in the same directory. Then

> rmarkdown::render("test.Rmd")

processing file: test.Rmd
  |...................................                                   |  50%
  ordinary text without R code

  |......................................................................| 100%
label: unnamed-chunk-1
Quitting from lines 10-11 (test.Rmd) 
Error in magick_image_readpath(enc2native(path), density, depth, strip) : 
  Magick: unable to open image 'test_files/figure-html/unnamed-chunk-1-1.png': No such file or directory @ error/blob.c/OpenBlob/3496

but removing

output:
  BiocStyle::html_document

results in

> rmarkdown::render("test.Rmd")

processing file: test.Rmd
  |...................................                                   |  50%
  ordinary text without R code

  |......................................................................| 100%
label: unnamed-chunk-1

output file: test.knit.md

/usr/local/bin/pandoc +RTS -K512m -RTS test.utf8.md --to html4 --from markdown+autolink_bare_uris+tex_math_single_backslash+smart --output test.html --email-obfuscation none --self-contained --standalone --section-divs --template /Users/ma38727/Library/R/4.0/Bioc/3.11/library/rmarkdown/rmd/h/default.html --no-highlight --variable highlightjs=1 --variable 'theme:bootstrap' --include-in-header /var/folders/yn/gmsh_22s2c55v816r6d51fx1tnyl61/T//Rtmp6dbVLW/rmarkdown-str8a247089c054.html --mathjax --variable 'mathjax-url:https://mathjax.rstudio.com/latest/MathJax.js?config=TeX-AMS-MML_HTMLorMML' --lua-filter /Users/ma38727/Library/R/4.0/Bioc/3.11/library/rmarkdown/rmd/lua/pagebreak.lua --lua-filter /Users/ma38727/Library/R/4.0/Bioc/3.11/library/rmarkdown/rmd/lua/latex-div.lua 

Output created: test.html
>

and test.html includes the figure.

matthewcarlucci commented 4 years ago

This issue is also causing build errors on biomaRt, BiocWorkflowTools, DiscoRhythm and likely many others using knitr::include_graphics().

hpages commented 4 years ago

Reported as a knitr issue with an example that removes BiocStyle from the equation.

lcolladotor commented 4 years ago

Thanks for reporting these two issues ^^

It's been affecting derfinder, recount and other BioC packages I made. Since it looks fixed on the knitr side, I guess this one can be closed too.

hpages commented 4 years ago

Although the fix will only become effective once the fixed version of knitr makes it to CRAN. I don't know how long this will take.

lcolladotor commented 4 years ago

We could edit our DESCRIPTION files with

remotes: yihui/knitr

as detailed at https://stackoverflow.com/questions/30493388/create-an-r-package-that-depends-on-another-r-package-located-on-github. Though maybe it makes more sense to edit BiocStyle's DESCRIPTION only as a temporary fix which should help other BioC packages that use BiocStyle, though it won't fix those that use knitr without BiocStyle... hm...

hpages commented 4 years ago

The build system ignores the remotes field so using remotes: yihui/knitr won't help.

What we could do is manually install the latest knitr from GitHub on all the builders. I'm just hoping that a new version of knitr will soon show up on CRAN so we don't need to do this.

lcolladotor commented 4 years ago

Ahhh, I didn't know that the build system ignores remotes. Oops!

Sounds like we need to ping Yihui then, right?

hpages commented 4 years ago

He already uploaded a new version of knitr to CRAN last Thursday (version 1.27) and I think that CRAN's policy doesn't let you upload a new version more than once a week or something like that so we might just need to wait.

In the meantime, after discussing with @mtmorgan and @lshep , we've decided to manually update knitr on the 6 build machines (well, 5 only since celaya2 is dead at the moment).