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

Display all plots in a for loop (knitted roxygen2 documentation) #2392

Open
grantmcdermott opened this issue Feb 4, 2025 · 12 comments
Open

Comments

@grantmcdermott
Copy link

Hi @yihui,

I'm the maintainer of tinyplot, a lightweight extension of the base R graphics system. One of the extensions that we have introduced is compatible themes for convenient aesthetic styling. The help documentation provides various examples. including this final command to cycle through the available themes:

#' # Themes showcase
#' ## We'll use a slightly more intricate plot (long y-axis labs and facets)
#' ## to demonstrate dynamic margin adjustment etc.
#' 
#' thms = eval(formals(tinytheme)$theme)
#' 
#' for (thm in thms) {
#'   tinytheme(thm)
#'   tinyplot(
#'     I(Sepal.Length*1e4) ~ Petal.Length | Species, facet = "by", data = iris,
#'     main = "Demonstration of tinyplot themes",
#'     sub = paste0('tinytheme("', thm, '")')
#'   )
#' }

(Source code is here.)

This approach works as expected when running the examples in the console. Unfortunately, the rendered package website only displays the very final theme; see https://grantmcdermott.com/tinyplot/man/tinytheme.html#examples

Having stepped through the various rendering steps, I'm weakly confident that this is ultimately a knitr issue (rather than, say, a roxygen2 or quarto issue). I have read through some older issues like #408, but they don't quite address my problem.

Do you perhaps have any suggestions for how to ensure that all the plots in this example render in the knitted website?

Many thanks in advance and also for the many excellent R packages that you maintain.

@yihui
Copy link
Owner

yihui commented Feb 4, 2025

@grantmcdermott First of all, I absolutely love tinyplot!

This issue might belong to pkgdown. I'm not 100% sure since I don't know how pkgdown runs examples, but I'm reasonably sure litedown should work fine: https://yihui.org/litedown/#sec:package-sites-via-github-action (I can verify it tomorrow morning)

@grantmcdermott
Copy link
Author

grantmcdermott commented Feb 4, 2025

Super, thanks @yihui. We actually use altdoc instead of pkgdown (and render to a Quarto website), but perhaps the mechanism is the same.

I've been keeping half an eye out on litedown. It looks very cool :-)

@etiennebacher
Copy link

Hi there, I just want to clarify this is not related to altdoc or pkgdown. If you have the following foo.Rmd:

---
title: "hello there"
---

```{r}
library(tinyplot)
thms <- eval(formals(tinytheme)$theme)

tinytheme("ipsum")
tinyplot(I(Sepal.Length * 1e4) ~ Petal.Length | Species, data = iris)

for (thm in thms) {
  tinytheme(thm)
  tinyplot(I(Sepal.Length * 1e4) ~ Petal.Length | Species, data = iris)
}
```

then:

  • litedown::fuse("foo.Rmd") outputs all plots
  • rmarkdown::render("foo.Rmd", output = "html_document") and quarto::quarto_render("foo.Rmd", output = "html") only print the output of the last iteration of the loop.

@cderv
Copy link
Collaborator

cderv commented Feb 6, 2025

rmarkdown::render("foo.Rmd", output = "html_document") and quarto::quarto_render("foo.Rmd", output = "html") only print the output of the last iteration of the loop.

This is definitely knitr + evaluate behavior as you can reproduce by knitr::knit("test.Rmd") which both rmarkdown and quarto uses.

@yihui do you know exactly the reason / design in knitr which you change in fuse ?

I believe this is limitation of knitr and recommended way to output several content like that is to use child document (https://bookdown.org/yihui/rmarkdown-cookbook/child-document.html)

Here is what would work in rmarkdown and quarto right now

```{r}
#| results: asis
res <- lapply(thms, function(thm) {
  res <- knitr::knit_child(text = c(
  "```{r echo = FALSE}",
  "tinytheme(thm)",
  "tinyplot(I(Sepal.Length * 1e4) ~ Petal.Length | Species, data = iris)",
  "```"
  ), envir = environment(), quiet = TRUE)
})

cat(unlist(res), sep = "\n")
```

@yihui
Copy link
Owner

yihui commented Feb 6, 2025

@etiennebacher Thanks! That's super helpful!

@cderv I think the problem comes from the evaluate package, which knitr uses to run code and capture plots. Due to the way it works (via recordPlot() in memory and comparing consecutive plot objects to figure out which plot to keep or discard), it can't guarantee a faithful capture of all possible plots, whereas litedown is guaranteed to capture all plots since it records plots via a graphical device to files directly, and there are no intermediate recordPlot() objects to analyze.

That said, this problem might be fixable in evaluate. I'm not 100% sure---it depends on whether the par() changes (introduced by tinytheme() in this case) between two plot objects can be reliably detected and we can also reliably make the decision whether the changes indicate new plots. Please feel free to file an issue to https://github.com/r-lib/evaluate and see if it could be fixed there.

@cderv
Copy link
Collaborator

cderv commented Feb 6, 2025

Thanks @yihui - really helpful ! I see what you are mentionning about record plot. I would think fig.keep='all' would have help, but it is not. I'll have a look.

At first, I thought this was related to the fact that knit_print() is called on last results really, not from within a for loop.

For example, this won't print anything as this is inside a for loop.

```{r}
for (thm in letters[1:5]) {
  knitr::asis_output(paste0("## ", thm, "\n"))
}
```

Maybe evaluate related too. I'll have a look

@yihui
Copy link
Owner

yihui commented Feb 6, 2025

It's true that for knit_print() to work, the value must be produced at the top level in the code. Plots don't go through knit_print(), so they can be produced anywhere in the code. I think evaluate has trimmed all previous plots in this case since these plots only differ in par().

@cderv
Copy link
Collaborator

cderv commented Feb 6, 2025

I think the plot issue is related to

which was recently closed.

Basically

res <- evaluate::evaluate(function(){ 
    library(tinyplot)
    tinytheme("dark")
    tinyplot(I(Sepal.Length * 1e4) ~ Petal.Length | Species, data = iris)
    tinytheme("minimal")
    tinyplot(I(Sepal.Length * 1e4) ~ Petal.Length | Species, data = iris)
})

will see all the plots

res <- evaluate::evaluate(function(){ 
    library(tinyplot)
    for (thm in c("dark", "minimal")) {
        tinytheme(thm)
        tinyplot(I(Sepal.Length * 1e4) ~ Petal.Length | Species, data = iris)
    }
})

will see only the last one.

So definitely related to for-loop context and not just plot change detection I think..

@yihui
Copy link
Owner

yihui commented Feb 6, 2025

Oh wait a minute. These plots also differ in sub titles, not only in par(). Hmm. The bug may be a little deeper than I thought.

BTW, here is a quick preview of the litedown site (I'll delete it later), and the example worked fine: https://git.yihui.org/tinyplot/manual.html#sec:examples_5:~:text=p()-,%23%20Themes%20showcase,-%23%23%20We%27ll%20use%20a

@etiennebacher
Copy link

These plots also differ in sub titles, not only in par()

In the example of the original post yes, but not in the simplified example I added later.

@grantmcdermott
Copy link
Author

Thanks everyone for diving deep on this. I really appreciate it.

Just a HU that I'm going to be on vacation (and very likely off grid) for the next week. So I won't be very responsive for the next few days... but hopefully @vincentarelbundock or @zeileis can reply if anything important on the tinyplot side comes up.

@cderv
Copy link
Collaborator

cderv commented Feb 10, 2025

Thanks @grantmcdermott .

I am currently trying to understand the difference with base plot system, and a minimal reproducible example in r-lib/evaluate#237 so that we can understand what is really going on and if this is at the evaluate level or not.

Any help appreciated

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

No branches or pull requests

4 participants