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

dev metadata key not honoured #368

Open
3 tasks done
fkohrt opened this issue Mar 30, 2023 · 2 comments
Open
3 tasks done

dev metadata key not honoured #368

fkohrt opened this issue Mar 30, 2023 · 2 comments
Labels

Comments

@fkohrt
Copy link

fkohrt commented Mar 30, 2023

It seems that the dev key that can be set as part of the YAML frontmatter is not being passed on to rmarkdown::html_document(). The following R Markdown document creates a Xaringan presentation with PNG graphics instead of SVG graphics:

---
title: SVG Figure Test
output:
  xaringan::moon_reader:
    dev: "svg"
---

```{r}
#| pie,
#| echo = FALSE,
#| fig.cap = "The only acceptable use of a pie chart.",
#| fig.alt = "A pie chart with slices of 78%, 17% and 6%. The positioning and coloring of the slices create the impression of a three-dimensional pyramid with a sunny and a shady side and a blue sky above."

# see https://github.com/rstudio/blogdown/blob/8719aa7/inst/resources/hello-world.Rmd#L27-L35
# and https://svn.r-project.org/R/tags/R-3-3-0/src/library/graphics/man/pie.Rd
par(mar = c(0, 1, 0, 1))
pie(
  c("Sky" = 280, "Sunny side of pyramid" = 60, "Shady side of pyramid" = 20),
  col = c('#0292D8', '#F7EA39', '#C4B632'),
  init.angle = -50, border = NA
)
```

Only setting the chunk option dev = "svg" (or using knitr::opts_chunk$set(dev = "svg")) creates SVG figures.

xfun::session_info("xaringan")
R version 4.2.1 (2022-06-23)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 18.04.6 LTS, RStudio 2022.2.1.461

Locale:
  LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C              
  LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8    
  LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8   
  LC_PAPER=en_US.UTF-8       LC_NAME=C                 
  LC_ADDRESS=C               LC_TELEPHONE=C            
  LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       

Package version:
  base64enc_0.1.3   bslib_0.4.2       cachem_1.0.7     
  cli_3.6.1         digest_0.6.31     ellipsis_0.3.2   
  evaluate_0.20     fastmap_1.1.1     fontawesome_0.5.0
  fs_1.6.1          glue_1.6.2        graphics_4.2.1   
  grDevices_4.2.1   highr_0.10        htmltools_0.5.5  
  httpuv_1.6.9      jquerylib_0.1.4   jsonlite_1.8.4   
  knitr_1.42        later_1.3.0       lifecycle_1.0.3  
  magrittr_2.0.3    memoise_2.0.1     methods_4.2.1    
  mime_0.12         promises_1.2.0.1  R6_2.5.1         
  rappdirs_0.3.3    Rcpp_1.0.10       rlang_1.1.0      
  rmarkdown_2.21    sass_0.4.5        servr_0.26       
  stats_4.2.1       stringi_1.7.12    stringr_1.5.0    
  tinytex_0.44      tools_4.2.1       utils_4.2.1      
  vctrs_0.6.1       xaringan_0.28.1   xfun_0.38        
  yaml_2.3.7       

By filing an issue to this repo, I promise that

  • I have fully read the issue guide at https://yihui.org/issue/.
  • I have provided the necessary information about my issue.
    • If I'm asking a question, I have already asked it on Stack Overflow or RStudio Community, waited for at least 24 hours, and included a link to my question there.
    • If I'm filing a bug report, I have included a minimal, self-contained, and reproducible example, and have also included xfun::session_info('xaringan'). I have upgraded all my packages to their latest versions (e.g., R, RStudio, and R packages), and also tried the development version: remotes::install_github('yihui/xaringan').
    • If I have posted the same issue elsewhere, I have also mentioned it in this issue.
  • I have learned the Github Markdown syntax, and formatted my issue correctly.

I understand that my issue may be closed if I don't fulfill my promises.

@cderv
Copy link
Collaborator

cderv commented Apr 7, 2023

Thanks for the report.

You're right that something is not working as we could expected. Your workarounds are the good ones.
here is some explanation to better understand so that we see what we could do.

When setting dev in the format function, it is passed from the format function to the knitr opts_chunk option. From the code when setting xaringan::moon_reader(dev = "svg"), it goes into ... argument part of the function and it is correctly passed to html_document

xaringan/R/render.R

Lines 186 to 188 in 8dcfa3f

rmarkdown::html_document(
..., includes = includes, mathjax = mathjax, pandoc_args = pandoc_args
)

This means that rmarkdown::html_document(dev = "svg") is correctly called. This will create the format function and use rmarkdown::knitr_options() to pass information to knitr::opts_chunk for you. However, this does not go as expected

rmarkdown::html_document(dev = "svg")$knitr$opts_chunk$dev
#> [1] "svg"
xaringan::moon_reader(dev = "svg")$knitr$opts_chunk$dev
#> NULL

This is because xaringan::moon_reader defines also knitr_options()

xaringan/R/render.R

Lines 210 to 211 in 8dcfa3f

rmarkdown::output_format(
rmarkdown::knitr_options(knit_hooks = highlight_hooks),

Those options are merged with the base format (which is the one receiving the dev config, and we hit a known merging limitation similar to rstudio/rmarkdown#1558

This is what happens internally

base_format_do <- rmarkdown::knitr_options(opts_chunk = list(dev = "svg"))
xaringan_format_do <- rmarkdown::knitr_options(NULL)
str(base_format_do)
#> List of 5
#>  $ opts_knit    : NULL
#>  $ opts_chunk   :List of 1
#>   ..$ dev: chr "svg"
#>  $ knit_hooks   : NULL
#>  $ opts_hooks   : NULL
#>  $ opts_template: NULL
str(xaringan_format_do)
#> List of 5
#>  $ opts_knit    : NULL
#>  $ opts_chunk   : NULL
#>  $ knit_hooks   : NULL
#>  $ opts_hooks   : NULL
#>  $ opts_template: NULL
str(rmarkdown:::merge_lists(base_format_do, xaringan_format_do))
#> List of 5
#>  $ opts_knit    : NULL
#>  $ opts_chunk   : NULL
#>  $ knit_hooks   : NULL
#>  $ opts_hooks   : NULL
#>  $ opts_template: NULL

@yihui this is our usual "merge output format" limitation due do initial design - other related issue are rstudio/rmarkdown#2407 and rstudio/rmarkdown#1558 (that I rememenber)

Usual way to fix this is to pass the argument that goes into knitr_options() for each format (meaning adding dev explicitly to xaringan::moon_reader() and not pass it to the base format. But that is "workaround" fix.

As merge_list consider NULL as a set value in overlay format, even when you don't want to override it from base it will be. I always wanted to fix this in R Markdown but any solution I think of (like using I() to mark default value) could be big breaking change (because I believe some formats rely on "IMO wrong" behavior.

More generally this is an issue when a format uses knitr = rmarkdown::knitr_options(...) and pandoc = rmarkdown::pandoc_options(...) in rmarkdown::output_format() as anything not modify will be a NULL seen as set value to use in overlay instead of base.

Hope it is clear.

Do you want to support this better in xaringan by adding explicitly more argument to xaringan::moon_reader() ? Or should we try to find a better way (if possible) to solve this in rmarkdown ?

@cderv cderv added the bug label Apr 7, 2023
@yihui
Copy link
Owner

yihui commented Apr 17, 2023

I always wanted to fix this in R Markdown but any solution I think of (like using I() to mark default value) could be big breaking change (because I believe some formats rely on "IMO wrong" behavior.

I tend to make this breaking change. We can do a PR and check revdeps first, to get an idea about how bad this change could be to other packages.

Do you want to support this better in xaringan by adding explicitly more argument to xaringan::moon_reader() ? Or should we try to find a better way (if possible) to solve this in rmarkdown ?

I tend not to add a new argument to moon_reader() because workarounds exist. If we can fix it in rmarkdown, that will certainly be great. If not, I feel knitr::opts_chunk$set(dev = "svg") in a setup chunk is not a bad workaround.

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

No branches or pull requests

3 participants