yihui / knitr

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

Figure sizing options don't work as vectors #2309

Closed svraka closed 7 months ago

svraka commented 7 months ago

According to the docs:

The chunk options dev, fig.ext, fig.width, fig.height, and dpi can be vectors (shorter ones will be recycled), e.g., dev = c('pdf', 'png') creates a PDF and a PNG file for the same plot.

However, in this example both plots will be 12 high.

---
title: fig.height
output: html_document
---

```{r figs}
#| fig.height = c(6, 12)
plot(iris[, 1:2])
plot(cars)

After digging into the code, I think `.recyle.opts` defines recyclable options and based on the git log, it never even contained `fig.height` (or any of the other figure sizing options).

https://github.com/yihui/knitr/blob/4b47d2a57bd6ac31712d0afafbc4fe968b663c92/R/plot.R#L267-L270

Am I missing something here?

# Session info

``` r
xfun::session_info('knitr')
xfun::session_info('knitr')
R version 4.3.2 (2023-10-31 ucrt)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 19045)

Locale:
  LC_COLLATE=Hungarian_Hungary.utf8  LC_CTYPE=Hungarian_Hungary.utf8   
  LC_MONETARY=Hungarian_Hungary.utf8 LC_NUMERIC=C                      
  LC_TIME=Hungarian_Hungary.utf8    

Package version:
  evaluate_0.23   graphics_4.3.2  grDevices_4.3.2 highr_0.10      knitr_1.45      methods_4.3.2  
  stats_4.3.2     tools_4.3.2     utils_4.3.2     xfun_0.41       yaml_2.3.7     
> 

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 7 months ago

I believe this is were we are vectorizing and creating several plots if we have several values in options https://github.com/yihui/knitr/blob/4b47d2a57bd6ac31712d0afafbc4fe968b663c92/R/output.R#L587-L593

This is not specifically doing what you expect and we may have issues / limitations.

I believe the doc you linked to refers to a way to create two plot outputs for the same plot code. "dev = c('pdf', 'png') creates a PDF and a PNG file for the same plot."

This does happen, but we do use the same file name for the plot file - so this will only works ok when different dev or fig.ext is used too

For example, this will produce 4 plots file inside the intermediate dir, two for each in the code chunk,

---
title: fig.height
output: html_document
---

```{r figs}
#| fig.height = c(6, 12),
#| fig.ext = c("1.png", "2.png")
plot(iris[, 1:2])
plot(cars)

@yihui should we somehow make the naming a variant of other vectorized argument too ? 

Though, I believe you are looking for something different. What you would like is applying different `fig.*` option to different plot in the chunk. As you pointed out, this is not recycled so. `dev` is not either. 

````markdown
---
title: fig.height
output: 
  html_document:
    self_contained: false
---

```{r figs}
#| fig.height = c(6, 12),
#| dev = c("png", "svg")
plot(iris[, 1:2])
plot(cars)

This will produce 4 plots, two for each plot in the chunk
````r
> fs::dir_tree("test_files/figure-html/")
test_files/figure-html/
├── figs-1.png
├── figs-1.svg
├── figs-2.png
└── figs-2.svg

So this is not about sizing only but how figure options are applied, differently from what you understand in the document.

dev, fig.ext, fig.width, fig.height, and dpi are correctly used and recycled for all plots, but also applied for all as a matrix (mapply()) is used.

I am not sure you can do only what you are expecting... without manual intervention to load the correct one of produced plot (knitr::fig_chunk() helping here maybe) or using another chunk for the other plot of different size. 🤔

yihui commented 7 months ago

@svraka The chunk options in .recyle.opts are vectorized over all plots in a code chunk, e.g., the first fig.cap is applied to the first plot, and the second fig.cap is applied to the second, etc.

On the other hand, chunk options like fig.height are vectorized over a single plot in a code chunk to create multiple copies of the same plot. @cderv has explained the reason why you only get 12in plots: in theory, four plot files would be created, but your dev = 'png' made the 12in plot overwrite the 6in plot, since both the 12in and 6in plot files have the same filename. You will have to specify different fig.ext to avoid the overwriting.

@yihui should we somehow make the naming a variant of other vectorized argument too ?

Maybe. I'm not sure what would be an appropriate naming scheme. Alternatively, we can issue a warning when potential overwriting is detected.

cderv commented 7 months ago

I'm not sure what would be an appropriate naming scheme.

I was thinking of only a counter - not great name but would avoid this.

Alternatively, we can issue a warning when potential overwriting is detected.

This would be at least useful to let user be aware of this. Though, there should be a way to remove it by providing a differentiation in name. Currently I believe only fig.ext like a I did allow this for arbitrary. Should there be a suffix option so that user can set it ?

If we can do a counter this seems ok.

Otherwise, naming could be include the set of options used when not impacting the name

<name>-<fig.width>w-<fig.height>h-<dpi>-<dev><fig.ext>

Only when the vectorization creates conflict.

🤷‍♂️ just ideas.

svraka commented 7 months ago

@svraka The chunk options in .recyle.opts are vectorized over all plots in a code chunk, e.g., the first fig.cap is applied to the first plot, and the second fig.cap is applied to the second, etc.

On the other hand, chunk options like fig.height are vectorized over a single plot in a code chunk to create multiple copies of the same plot. @cderv has explained the reason why you only get 12in plots: in theory, four plot files would be created, but your dev = 'png' made the 12in plot overwrite the 6in plot, since both the 12in and 6in plot files have the same filename. You will have to specify different fig.ext to avoid the overwriting.

Thanks for the explanation, that makes sense! The docs could be expanded with this. Just to avoid any surprises, I wouldn't suggest any changes to knitr's behaviour.

On second thought, I even changed my real code so I don't need to set multiple fig.heights in a chunk. I had something like this, where plots is a data.frame with two columns, a character vector used as captions, and a list of plots:

```{r plots}
#| fig.cap: !expr plots[["caption"]]
invisible(lapply(plots[["plot"]], print))


These plots have mostly similar structures, except one, which needed different sizing. I ended up splitting the chunk into two, one for the special case, one for the rest. It is more readable anyway.
yihui commented 7 months ago

Okay, I just updated the documentation to clarify. Thanks!

github-actions[bot] commented 4 weeks 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.