yihui / knitr

A general-purpose tool for dynamic report generation in R
https://yihui.org/knitr/
2.36k stars 873 forks source link

Improve `knitr::spin` rendering with `.Rmd` files #2281

Closed kylebutts closed 10 months ago

kylebutts commented 10 months ago

I recently discovered knitr::spin which I agree is knitr's best hidden gem!

The current version uses knit2html which (1) raises warning that rmarkdown::render should be used and (2) ignores the output yaml option: https://github.com/yihui/knitr/blob/50e28dab843109d5292e26695d1b486b36eec0e2/R/spin.R#L119-L121

A proposed solution that works with both text and hair options

if (format == 'Rmd') {
  if (is.null(outsrc)) {
    knit2html(outsrc, text = txt, envir = envir) 
  } else {
    rmarkdown::render(outsrc, envir = envir)
  }
} else if (!is.null(outsrc) && (format %in% c('Rnw', 'Rtex'))) { 

Would you welcome a PR adding this change?


By filing an issue to this repo, I promise that

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

cderv commented 10 months ago

Documentation of the function says https://github.com/yihui/knitr/blob/50e28dab843109d5292e26695d1b486b36eec0e2/R/spin.R#L44-L47

Try using rmarkdown::render() on the .R file directly. See documentation at https://rmarkdown.rstudio.com/docs/reference/compile_notebook.html

kylebutts commented 10 months ago

Thanks @cderv. I hadn't seen that in the documentation but pieced it together. Why not have the default be to use render which seems more robust (using v2) and lets you specify output format?

cderv commented 10 months ago

I'll let @yihui answer to that.

kylebutts commented 10 months ago

I originally found about this function from the R Markdown Cookbook: https://bookdown.org/yihui/rmarkdown-cookbook/spin.html

This example would suggest you get a pdf document out, but you get an html

cderv commented 10 months ago

Thanks for the context, this is indeed confusing...

@yihui as anything changed since we wrote the R Markdown cookbook ?

rmarkdown::render() on R file seems the way to spin and create the report with Rmd format. Should knitr::spin() offer to do rmarkdown::render() if a Rmd file with R Markdown V2 is detected ?

Otherwise, the cookbook should be updated 🤔

yihui commented 10 months ago

Why not have the default be to use render which seems more robust (using v2) and lets you specify output format?

@kylebutts knitr::spin() was introduced before the rmarkdown package was born, so it uses knitr::knit2html() instead of rmarkdown::render() mainly for historical reasons.

This example would suggest you get a pdf document out, but you get an html

We need to clarify in the book that the R script is supposed to be rendered via rmarkdown::render() instead of knitr::spin() (spin() is only an intermediate step). Thanks!

Should knitr::spin() offer to do rmarkdown::render() if a Rmd file with R Markdown V2 is detected ?

@cderv That's probably a good idea. Anyway, the cookbook definitely needs a bit more clarification.

github-actions[bot] commented 4 months ago

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue by following the issue guide (https://yihui.org/issue/), and link to this old issue if necessary.