Skip to content

Commit

Permalink
Add param seed to build_articles() (r-lib#2354)
Browse files Browse the repository at this point in the history
Ensures reproducible RNG, reducing noise in final HTML output.
  • Loading branch information
salim-b authored and SebKrantz committed Jun 1, 2024
1 parent 816430c commit a7c5d58
Show file tree
Hide file tree
Showing 13 changed files with 85 additions and 29 deletions.
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
* Remove redundant entries in the documentation index when multiple explicit `@usage` tags are provided (@klmr, #2302)
* The article index now sorts vignettes and non-vignette articles alphabetically by their filename (literally, their `basename()`), by default (@jennybc, #2253).
* Add Catalan translation (@jmaspons, #2333)
* Set RNG seed before building articles by default. Use `build_articles(seed = NULL)` for the old (unreproducible) behaviour. (@salim-b, #2354).
* Set RNG seed for htmlwidgets IDs. This reduces noise in final HTML output,
both for articles and examples in the reference that contain htmlwidgets
(@salim-b, #2294, #2354).
* Set RNG seed for htmlwidgets IDs. This reduces noise in pkgdown reference HTML output when examples generate htmlwidgets (@salim-b, #2294).
* Fix BS5 navbar template to get `navbar.type: dark` to work with bslib 0.6+ / Bootstrap 5.3+ (@tanho63, #2388)
* Allow detection of quarto `.qmd` articles and let them be processed using the [`quarto`](https://cran.r-project.org/web/packages/quarto/index.html) vignette builder (@rcannood, #2404).
Expand Down
22 changes: 15 additions & 7 deletions R/build-articles.R
Original file line number Diff line number Diff line change
Expand Up @@ -163,12 +163,15 @@
#' pandoc. This is useful when debugging.
#' @param lazy If `TRUE`, will only re-build article if input file has been
#' modified more recently than the output file.
#' @param seed Seed used to initialize random number generation in order to
#' make article output reproducible. An integer scalar or `NULL` for no seed.
#' @param preview If `TRUE`, or `is.na(preview) && interactive()`, will preview
#' freshly generated section in browser.
#' @export
build_articles <- function(pkg = ".",
quiet = TRUE,
lazy = TRUE,
seed = 1014L,
override = list(),
preview = NA) {
pkg <- section_init(pkg, depth = 1L, override = override)
Expand All @@ -181,10 +184,12 @@ build_articles <- function(pkg = ".",

build_articles_index(pkg)
purrr::walk(
pkg$vignettes$name, build_article,
pkg$vignettes$name,
build_article,
pkg = pkg,
quiet = quiet,
lazy = lazy
lazy = lazy,
seed = seed,
quiet = quiet
)

preview_site(pkg, "articles", preview = preview)
Expand All @@ -196,10 +201,12 @@ build_articles <- function(pkg = ".",
#' relative to `vignettes/` without extension, or `index` or `README`.
#' @param data Additional data to pass on to template.
build_article <- function(name,
pkg = ".",
data = list(),
lazy = FALSE,
quiet = TRUE) {
pkg = ".",
data = list(),
lazy = FALSE,
seed = 1014L,
quiet = TRUE) {

pkg <- as_pkgdown(pkg)

# Look up in pkg vignette data - this allows convenient automatic
Expand Down Expand Up @@ -285,6 +292,7 @@ build_article <- function(name,
output = output_file,
output_format = format,
output_options = options,
seed = seed,
quiet = quiet
)
}
Expand Down
6 changes: 2 additions & 4 deletions R/build-reference.R
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,6 @@
#' rapidly prototype. It is set to `FALSE` by [build_site()].
#' @param run_dont_run Run examples that are surrounded in \\dontrun?
#' @param examples Run examples?
#' @param seed Seed used to initialize random number generation so that
#' examples are reproducible.
#' @param devel Determines how code is loaded in order to run examples.
#' If `TRUE` (the default), assumes you are in a live development
#' environment, and loads source package with [pkgload::load_all()].
Expand All @@ -153,7 +151,7 @@ build_reference <- function(pkg = ".",
lazy = TRUE,
examples = TRUE,
run_dont_run = FALSE,
seed = 1014,
seed = 1014L,
override = list(),
preview = NA,
devel = TRUE,
Expand Down Expand Up @@ -210,7 +208,7 @@ copy_figures <- function(pkg) {
}
}

examples_env <- function(pkg, seed = 1014, devel = TRUE, envir = parent.frame()) {
examples_env <- function(pkg, seed = 1014L, devel = TRUE, envir = parent.frame()) {
# Re-loading pkgdown while it's running causes weird behaviour with
# the context cache
if (isTRUE(devel) && !(pkg$package %in% c("pkgdown", "rprojroot"))) {
Expand Down
6 changes: 3 additions & 3 deletions R/build.R
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@
build_site <- function(pkg = ".",
examples = TRUE,
run_dont_run = FALSE,
seed = 1014,
seed = 1014L,
lazy = FALSE,
override = list(),
preview = NA,
Expand Down Expand Up @@ -375,7 +375,7 @@ build_site <- function(pkg = ".",
build_site_external <- function(pkg = ".",
examples = TRUE,
run_dont_run = FALSE,
seed = 1014,
seed = 1014L,
lazy = FALSE,
override = list(),
preview = NA,
Expand Down Expand Up @@ -417,7 +417,7 @@ build_site_external <- function(pkg = ".",
build_site_local <- function(pkg = ".",
examples = TRUE,
run_dont_run = FALSE,
seed = 1014,
seed = 1014L,
lazy = FALSE,
override = list(),
preview = NA,
Expand Down
17 changes: 15 additions & 2 deletions R/rmarkdown.R
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#' Render RMarkdown document in a fresh session
#'
#' @noRd
render_rmarkdown <- function(pkg, input, output, ..., copy_images = TRUE, quiet = TRUE) {
render_rmarkdown <- function(pkg, input, output, ..., seed = NULL, copy_images = TRUE, quiet = TRUE) {

input_path <- path_abs(input, pkg$src_path)
output_path <- path_abs(output, pkg$dst_path)
Expand All @@ -20,13 +20,26 @@ render_rmarkdown <- function(pkg, input, output, ..., copy_images = TRUE, quiet
intermediates_dir = tempdir(),
encoding = "UTF-8",
envir = globalenv(),
seed = seed,
...,
quiet = quiet
)

path <- tryCatch(
callr::r_safe(
function(...) rmarkdown::render(...),
function(seed, envir, ...) {
if (!is.null(seed)) {
# since envir is copied from the parent fn into callr::r_safe(),
# set.seed() sets the seed in the wrong global env and we have to
# manually copy it over
set.seed(seed)
envir$.Random.seed <- .GlobalEnv$.Random.seed
if (requireNamespace("htmlwidgets", quietly = TRUE)) {
htmlwidgets::setWidgetIdSeed(seed)
}
}
rmarkdown::render(envir = envir, ...)
},
args = args,
show = !quiet,
env = c(
Expand Down
13 changes: 12 additions & 1 deletion man/build_articles.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions man/build_reference.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions man/build_site.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions tests/testthat/_snaps/build-articles.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,10 @@
Reading vignettes/html-deps.Rmd
Writing articles/html-deps.html

# output is reproducible by default, i.e. 'seed' is respected

Code
cat(output)
Output
## [1] 0.080750138 0.834333037 0.600760886 0.157208442 0.007399441

7 changes: 7 additions & 0 deletions tests/testthat/assets/articles/vignettes/random.Rmd
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
title: "Random"
---

```{r, repro}
runif(5L)
```
13 changes: 13 additions & 0 deletions tests/testthat/test-build-articles.R
Original file line number Diff line number Diff line change
Expand Up @@ -215,3 +215,16 @@ test_that("check doesn't include getting started vignette", {

expect_error(data_articles_index(pkg), NA)
})

test_that("output is reproducible by default, i.e. 'seed' is respected", {
pkg <- local_pkgdown_site(test_path("assets/articles"))
suppressMessages(build_article(pkg = pkg, name = "random"))

output <- xml2::read_html(file.path(pkg$dst_path, "articles/random.html")) %>%
rvest::html_node("div.contents > pre") %>%
rvest::html_text() %>%
# replace line feeds with whitespace to make output platform independent
gsub("\r", "", .)

expect_snapshot(cat(output))
})
2 changes: 1 addition & 1 deletion tests/testthat/test-check.R
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ test_that("fails if article index incomplete", {
pkg <- local_pkgdown_site(test_path("assets/articles"), meta = "
articles:
- title: Title
contents: [starts_with('html'), standard, toc-false, widget]
contents: [starts_with('html'), random, standard, toc-false, widget]
")
expect_snapshot(check_pkgdown(pkg), error = TRUE)
})
Expand Down
5 changes: 0 additions & 5 deletions vignettes/test/widgets.Rmd
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ knitr::opts_chunk$set(
Test spacing above widget.

```{r, echo=FALSE}
# set seed for reproducible widget id
if (requireNamespace("htmlwidgets", quietly = TRUE)) {
htmlwidgets::setWidgetIdSeed(42)
}
path1 <- tempfile()
writeLines(letters, path1)
path2 <- tempfile()
Expand Down

0 comments on commit a7c5d58

Please sign in to comment.