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

Difference in behavior for before/after_chapter_script and new_session: true/false #1252

Open
singmann opened this issue Sep 3, 2021 · 3 comments

Comments

@singmann
Copy link

singmann commented Sep 3, 2021

Sorry for the chunky title, but this seems to be a bug then only shows up in a very specific situation: If using the Knit-Merge strategy (i.e., new_session: true in _bookdown.yml) and a before_chapter_script (e.g., _common.R), fig.width in the before_chapter_script seems to be ignored. This does not seem to apply to other chunk options as far as I can tell, but particularly to fig.width. fig.width can also still be used directly in the chunk and works, but the global one is ignored.

The easiest way to reproduce the bug is the following steps:

  1. Create a new project using the new bs4_book through the RStudio menu interface (New Project, ...). This ensures that _common.R is present and new_session: true.
  2. Add the following code chunk to the index.Rmd file:
    ```{r tmp}
    par(mar = c(4, 4, .1, .1))
    plot(pressure, type = 'b', pch = 19)
    ```
    
  3. Change the global knitr options in _common.R to:
    # example chunk options set globally
    knitr::opts_chunk$set(
      comment = "#>",
      collapse = TRUE, 
      out.width = "25%", 
      fig.width = 3, 
      fig.asp = 0.9
      )
    

Feel free to play around with fig.width and see that changing it does not change the width of the figure in the output, unless you change new_session: true to new_session: false in _bookdown.yml or set it directly in the chunk. As far as I can tell the other figure options changed here (i.e., out.width and fig.asp) seem to work.

Also, this seems to be related to a comment in #36, #36 (comment)_

It also seems that an example of the bug can be found in the wild in the current version of "R for data science". At the moment, the width of the first figure in the data visualisation chapter is 1344 pixels. However, it appears it should be 1152 pixels because the _common.R file sets the fig.width to 6 (at least when setting fig.width of a chunk to 7, I also get 1344, and with 6, 1152).

My system:

> xfun::session_info('bookdown')
R version 4.1.1 (2021-08-10)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 19043), RStudio 1.4.1707

Locale:
  LC_COLLATE=English_United Kingdom.1252 
  LC_CTYPE=English_United Kingdom.1252   
  LC_MONETARY=English_United Kingdom.1252
  LC_NUMERIC=C                           
  LC_TIME=English_United Kingdom.1252    

Package version:
  base64enc_0.1.3 bookdown_0.24.1 digest_0.6.27   evaluate_0.14  
  fastmap_1.1.0   glue_1.4.2      graphics_4.1.1  grDevices_4.1.1
  highr_0.9       htmltools_0.5.2 jquerylib_0.1.4 jsonlite_1.7.2 
  knitr_1.33      magrittr_2.0.1  markdown_1.1    methods_4.1.1  
  mime_0.11       rlang_0.4.11    rmarkdown_2.10  stats_4.1.1    
  stringi_1.7.4   stringr_1.4.0   tinytex_0.33    tools_4.1.1    
  utils_4.1.1     xfun_0.25       yaml_2.2.1 
@cderv
Copy link
Collaborator

cderv commented Sep 6, 2021

I believe this is related to another thread #1049
It is the second time that this situation is puzzling users since changes in #36 (comment) due to #405 and #852.

Below is an attempt to detail the current situation!

What happens when new_session: true compared to new_session: false ?

When new session is FALSE, before_chapter_script content will be inserted in the document to render. When new_session: TRUE, the behavior is a bit different. before_chapter_script will be sourced in the R session before render() happens. This can cause some issues with options set in render(). See #1049 discussion.
Regarding behavior, the former is "as-if" the option were set inside the Rmd document, whereas the latter is "as-if" the options are set in the R session before rendering happens. This difference has consequences, and not just with bookdown. This is because an output format will have some default configuration which will set options, and it is possible that those default will take precedence over sopme values defines in the current R session.

Regarding an option like fig.width, this is the case. Formats like html_document() will define some option, and fig.width is on of them (see https://pkgs.rstudio.com/rmarkdown/reference/html_document.html and the fig_width argument). See this example in test.Rmd:

---
title: fig.width
output: html_document
---

```{r}
knitr::opts_chunk$get("fig.width")
```
  • knit the Rmd. The value printed will be 7
  • Run
    rmarkdown::render("test.Rmd", rmarkdown::html_document(fig_width = 10))
    This will change the option to 10
  • Change the option before rendering
    # Changing the option
    knitr::opts_chunk$set(fig.width = 3)
    # Rendering again with default value
    rmarkdown::render("test.Rmd")
    The value will be 7 again - the default one for html_document() and not 3, the one defined in option.

So this behavior is not something specific to bookdown, it is how output formats and rmarkdown is working. Unfortunately, with bookdown and multi file project, it is easier to stumble upon it.
To clarify, bs4_book() is based on html_document and will inherit the configuration of html_document

What are the solution to this ?

First way is, for any option made configurable through the output format, to change the value from the output format directly. This means that you can do this

output:
  bookdown::bs4_book:
    fig_width: 3

and this would change the option for the whole book.

Another way is to explicitly run the before_chapter_script file as a chunk inside each Rmd file of the book, as you found in #36 (comment)

To sum up

As I said, the behavior is different since bookdown 0.18 to solve different issues with before/after_chapter_script when new_session: true. This led to this situations where it does not behave the same depending on new_session: true.

I believe that before/after_chapter_script was added by convenience but it hides what happens really. The best way could be to be explicit about it, like with any other Rmd project: Change the configuration in a chunk of the rendered document.

  • With new_session: false, this would be enough in index.Rmd first chunk
  • With new_session: true, as each Rmd is rendered on its own, this means reloading the configuration in each file using one of the above method.

@singmann does it helps understand how this currently works ?
Reverting back to behavior pre-0.18 would not be desirable, and I don't know if there is better solution than the one above, or better documentation on what options can be change at the output format level.

one way forward could be to allow changing knitr option globally for a project at the YAML level to merge with the output format. 🤔

@singmann
Copy link
Author

singmann commented Sep 6, 2021

Thanks for the detailed explanation and different solutions. That explains it to me. Furthermore, for my current project the possibility of specifying the size through the output format is completely sufficient. If I need more fine grained differentiation I would just do it chunk wise.

However, I guess one remaining issue is that the current behaviour (i.e., the different ways before_chapter_script behaves) does not appear to be documented at the usual places. For example, in the bookdown book I could not find this. I think good places could be either the configuration section or the two rendering approaches section.

Furthermore, the fact that the arguably most popular bookdown book, R4DS, also relies on this non-working behaviour maybe sets a bad example. I would assume I am probably not the only one basing ones configuration on the corresponding repository and then getting bitten by it.

In any case, from my perspective this solves the issue. But I will leave it to you to close or not. Maybe updating the documentation somewhere is enough to ensure that it does not lead to too many people going bug hunting (i.e., I spend some time to nail down the exact situation in which it happens).

@cderv
Copy link
Collaborator

cderv commented Sep 6, 2021

Yes updating the documentation seems a good thing to do, thanks !

@cderv cderv changed the title Global chunk option fig.width (i.e., in knitr::opts_chunk()) in _common.R is ignored if new_session: true Difference in behavior for before/after_chapter_script and new_session: true/false Sep 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants