From 7d0b1f3d0dd605fc44ac20dacc23a193028af537 Mon Sep 17 00:00:00 2001 From: Salim B <git@salim.space> Date: Thu, 19 Oct 2023 00:02:06 +0200 Subject: [PATCH 1/9] Add param `seed` to `build_articles()` Adds a param `seed` to `build_artiles()` that allows reproducible RNG, which can reduce noise in final HTML output. --- DESCRIPTION | 2 +- R/build-articles.R | 22 +++++++++++++++------- R/rmarkdown.R | 14 ++++++++++++-- man/build_articles.Rd | 13 ++++++++++++- man/build_site.Rd | 9 ++++++--- vignettes/test/widgets.Rmd | 5 ----- 6 files changed, 46 insertions(+), 19 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 25e38d08e..dc6c659fe 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -64,5 +64,5 @@ Config/potools/style: explicit Config/testthat/edition: 3 Encoding: UTF-8 Roxygen: list(markdown = TRUE) -RoxygenNote: 7.2.2.9000 +RoxygenNote: 7.2.3 SystemRequirements: pandoc diff --git a/R/build-articles.R b/R/build-articles.R index 32c60612d..ae4bc0033 100644 --- a/R/build-articles.R +++ b/R/build-articles.R @@ -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. #' @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 = 1014, override = list(), preview = NA) { pkg <- section_init(pkg, depth = 1L, override = override) @@ -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) @@ -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 = NULL, + quiet = TRUE) { + pkg <- as_pkgdown(pkg) # Look up in pkg vignette data - this allows convenient automatic @@ -283,6 +290,7 @@ build_article <- function(name, output = output_file, output_format = format, output_options = options, + seed = seed, quiet = quiet ) } diff --git a/R/rmarkdown.R b/R/rmarkdown.R index f13241862..4e024ef97 100644 --- a/R/rmarkdown.R +++ b/R/rmarkdown.R @@ -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) @@ -20,13 +20,23 @@ 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)) { + 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( diff --git a/man/build_articles.Rd b/man/build_articles.Rd index 7dda12caa..2c9c4389a 100644 --- a/man/build_articles.Rd +++ b/man/build_articles.Rd @@ -10,11 +10,19 @@ build_articles( pkg = ".", quiet = TRUE, lazy = TRUE, + seed = 1014, override = list(), preview = NA ) -build_article(name, pkg = ".", data = list(), lazy = FALSE, quiet = TRUE) +build_article( + name, + pkg = ".", + data = list(), + lazy = FALSE, + seed = NULL, + quiet = TRUE +) build_articles_index(pkg = ".") } @@ -27,6 +35,9 @@ pandoc. This is useful when debugging.} \item{lazy}{If \code{TRUE}, will only re-build article if input file has been modified more recently than the output file.} +\item{seed}{Seed used to initialize random number generation in order to +make article output reproducible.} + \item{override}{An optional named list used to temporarily override values in \verb{_pkgdown.yml}} diff --git a/man/build_site.Rd b/man/build_site.Rd index 7b5e80af7..21f1cecbf 100644 --- a/man/build_site.Rd +++ b/man/build_site.Rd @@ -25,8 +25,8 @@ build_site( \item{run_dont_run}{Run examples that are surrounded in \\dontrun?} -\item{seed}{Seed used to initialize random number generation so that -examples are reproducible.} +\item{seed}{Seed used to initialize random number generation in order to +make article output reproducible.} \item{lazy}{If \code{TRUE}, will only rebuild articles and reference pages if the source is newer than the destination.} @@ -199,7 +199,7 @@ This is the default structure: It makes use of the the six built-in components: \itemize{ -\item \code{intro}: "Get Started", which links to a vignette with the same name as the package. +\item \code{intro}: "Get Started", which links to a vignette with the same name as the package\link{^dots}. \item \code{reference}, if there are any \code{.Rd} files. \item \code{articles}, if there are any vignettes or articles. \item \code{tutorials}, if there any tutorials. @@ -208,6 +208,9 @@ It makes use of the the six built-in components: \item \code{github}, a link to the source repository (with an icon), if it can be automatically determined from the \code{DESCRIPTION}. } +\link{^dots}: Note that dots (\code{.}) in the package name need to be replaced by hyphens (\code{-}) in the vignette filename to be recognized as the intro. That means for a +package \code{foo.bar} the intro needs to be named \code{foo-bar.Rmd}. + You can use the \code{structure} field to reorganise the navbar without changing the default contents: \if{html}{\out{<div class="sourceCode yaml">}}\preformatted{navbar: diff --git a/vignettes/test/widgets.Rmd b/vignettes/test/widgets.Rmd index f4200cc90..b6c144a2d 100644 --- a/vignettes/test/widgets.Rmd +++ b/vignettes/test/widgets.Rmd @@ -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() From 2c2cdbd1a354d3165732b759eb64e82051a5bb9c Mon Sep 17 00:00:00 2001 From: Salim B <git@salim.space> Date: Thu, 19 Oct 2023 02:14:47 +0200 Subject: [PATCH 2/9] Tweak doc --- R/build-articles.R | 2 +- man/build_articles.Rd | 2 +- man/build_site.Rd | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/R/build-articles.R b/R/build-articles.R index ae4bc0033..3528a8f5f 100644 --- a/R/build-articles.R +++ b/R/build-articles.R @@ -164,7 +164,7 @@ #' @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. +#' 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 diff --git a/man/build_articles.Rd b/man/build_articles.Rd index 2c9c4389a..7a3ada456 100644 --- a/man/build_articles.Rd +++ b/man/build_articles.Rd @@ -36,7 +36,7 @@ pandoc. This is useful when debugging.} modified more recently than the output file.} \item{seed}{Seed used to initialize random number generation in order to -make article output reproducible.} +make article output reproducible. An integer scalar or \code{NULL} for no seed.} \item{override}{An optional named list used to temporarily override values in \verb{_pkgdown.yml}} diff --git a/man/build_site.Rd b/man/build_site.Rd index 21f1cecbf..0b307ed29 100644 --- a/man/build_site.Rd +++ b/man/build_site.Rd @@ -26,7 +26,7 @@ build_site( \item{run_dont_run}{Run examples that are surrounded in \\dontrun?} \item{seed}{Seed used to initialize random number generation in order to -make article output reproducible.} +make article output reproducible. An integer scalar or \code{NULL} for no seed.} \item{lazy}{If \code{TRUE}, will only rebuild articles and reference pages if the source is newer than the destination.} From 09d0e773a28b6ec69050700445b1296c98ba0e25 Mon Sep 17 00:00:00 2001 From: Salim B <git@salim.space> Date: Thu, 19 Oct 2023 02:16:05 +0200 Subject: [PATCH 3/9] Use explicit int seed --- R/build-articles.R | 2 +- R/build-reference.R | 4 ++-- R/build.R | 6 +++--- man/build_articles.Rd | 2 +- man/build_reference.Rd | 2 +- man/build_site.Rd | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/R/build-articles.R b/R/build-articles.R index 3528a8f5f..7e8b7677d 100644 --- a/R/build-articles.R +++ b/R/build-articles.R @@ -171,7 +171,7 @@ build_articles <- function(pkg = ".", quiet = TRUE, lazy = TRUE, - seed = 1014, + seed = 1014L, override = list(), preview = NA) { pkg <- section_init(pkg, depth = 1L, override = override) diff --git a/R/build-reference.R b/R/build-reference.R index add462c13..9dcc92f73 100644 --- a/R/build-reference.R +++ b/R/build-reference.R @@ -153,7 +153,7 @@ build_reference <- function(pkg = ".", lazy = TRUE, examples = TRUE, run_dont_run = FALSE, - seed = 1014, + seed = 1014L, override = list(), preview = NA, devel = TRUE, @@ -205,7 +205,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"))) { diff --git a/R/build.R b/R/build.R index e7ccef797..127ebe17d 100644 --- a/R/build.R +++ b/R/build.R @@ -321,7 +321,7 @@ build_site <- function(pkg = ".", examples = TRUE, run_dont_run = FALSE, - seed = 1014, + seed = 1014L, lazy = FALSE, override = list(), preview = NA, @@ -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, @@ -415,7 +415,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, diff --git a/man/build_articles.Rd b/man/build_articles.Rd index 7a3ada456..c7ff6142e 100644 --- a/man/build_articles.Rd +++ b/man/build_articles.Rd @@ -10,7 +10,7 @@ build_articles( pkg = ".", quiet = TRUE, lazy = TRUE, - seed = 1014, + seed = 1014L, override = list(), preview = NA ) diff --git a/man/build_reference.Rd b/man/build_reference.Rd index 730c2e2a3..5fc668be2 100644 --- a/man/build_reference.Rd +++ b/man/build_reference.Rd @@ -10,7 +10,7 @@ build_reference( lazy = TRUE, examples = TRUE, run_dont_run = FALSE, - seed = 1014, + seed = 1014L, override = list(), preview = NA, devel = TRUE, diff --git a/man/build_site.Rd b/man/build_site.Rd index 0b307ed29..2433cc28a 100644 --- a/man/build_site.Rd +++ b/man/build_site.Rd @@ -8,7 +8,7 @@ build_site( pkg = ".", examples = TRUE, run_dont_run = FALSE, - seed = 1014, + seed = 1014L, lazy = FALSE, override = list(), preview = NA, From b4d24554376c52b4cc4737fe263aef253e7f9ac3 Mon Sep 17 00:00:00 2001 From: Salim B <git@salim.space> Date: Thu, 19 Oct 2023 02:30:02 +0200 Subject: [PATCH 4/9] Add NEWS bullet --- NEWS.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 0a1ea665d..9f84f1a18 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,7 +4,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 for htmlwidgets IDs. This reduces noise in pkgdown reference HTML output when examples generate htmlwidgets (@salim-b, #2294). +* 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). # pkgdown 2.0.7 From bbc96292a12a85b4f62b963b4cd24797e039fbd2 Mon Sep 17 00:00:00 2001 From: Salim B <git@salim.space> Date: Mon, 11 Mar 2024 16:48:56 +0100 Subject: [PATCH 5/9] Default to `seed = 1014L` in `build_article()`, too --- R/build-articles.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/build-articles.R b/R/build-articles.R index c8262eff8..7780f01ae 100644 --- a/R/build-articles.R +++ b/R/build-articles.R @@ -204,7 +204,7 @@ build_article <- function(name, pkg = ".", data = list(), lazy = FALSE, - seed = NULL, + seed = 1014L, quiet = TRUE) { pkg <- as_pkgdown(pkg) From 94693b95fe4233fcaa5cdcca81e5568d0147f34e Mon Sep 17 00:00:00 2001 From: Salim B <git@salim.space> Date: Tue, 12 Mar 2024 00:52:29 +0100 Subject: [PATCH 6/9] Add comment about global env pitfall --- R/rmarkdown.R | 3 +++ 1 file changed, 3 insertions(+) diff --git a/R/rmarkdown.R b/R/rmarkdown.R index e131f670f..4aacf2a9a 100644 --- a/R/rmarkdown.R +++ b/R/rmarkdown.R @@ -29,6 +29,9 @@ render_rmarkdown <- function(pkg, input, output, ..., seed = NULL, copy_images = callr::r_safe( 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)) { From 595b41fafddae7f62d72f2bf65e443a7f2ed0b69 Mon Sep 17 00:00:00 2001 From: Salim B <git@salim.space> Date: Tue, 12 Mar 2024 00:52:51 +0100 Subject: [PATCH 7/9] Roxygenize --- man/build_articles.Rd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/build_articles.Rd b/man/build_articles.Rd index c7ff6142e..3f5b9c072 100644 --- a/man/build_articles.Rd +++ b/man/build_articles.Rd @@ -20,7 +20,7 @@ build_article( pkg = ".", data = list(), lazy = FALSE, - seed = NULL, + seed = 1014L, quiet = TRUE ) From 841d6d684210f1bccd22f712523e6cadf1520b11 Mon Sep 17 00:00:00 2001 From: Salim B <git@salim.space> Date: Tue, 12 Mar 2024 00:53:56 +0100 Subject: [PATCH 8/9] Test that articles are built reproducible by default --- DESCRIPTION | 1 + tests/testthat/_snaps/build-articles.md | 7 +++++++ tests/testthat/assets/articles/vignettes/random.Rmd | 7 +++++++ tests/testthat/test-build-articles.R | 13 +++++++++++++ tests/testthat/test-check.R | 2 +- 5 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 tests/testthat/assets/articles/vignettes/random.Rmd diff --git a/DESCRIPTION b/DESCRIPTION index 8940b9399..aa7501f1e 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -55,6 +55,7 @@ Suggests: rsconnect, rstudioapi, rticles, + rvest, sass, testthat (>= 3.1.3), tools diff --git a/tests/testthat/_snaps/build-articles.md b/tests/testthat/_snaps/build-articles.md index 186dd29ad..3a2482f5a 100644 --- a/tests/testthat/_snaps/build-articles.md +++ b/tests/testthat/_snaps/build-articles.md @@ -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 + diff --git a/tests/testthat/assets/articles/vignettes/random.Rmd b/tests/testthat/assets/articles/vignettes/random.Rmd new file mode 100644 index 000000000..a703ecae6 --- /dev/null +++ b/tests/testthat/assets/articles/vignettes/random.Rmd @@ -0,0 +1,7 @@ +--- +title: "Random" +--- + +```{r, repro} +runif(5L) +``` diff --git a/tests/testthat/test-build-articles.R b/tests/testthat/test-build-articles.R index 037702796..95b32df67 100644 --- a/tests/testthat/test-build-articles.R +++ b/tests/testthat/test-build-articles.R @@ -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)) +}) diff --git a/tests/testthat/test-check.R b/tests/testthat/test-check.R index e3a9edb6b..906e45150 100644 --- a/tests/testthat/test-check.R +++ b/tests/testthat/test-check.R @@ -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) }) From 40479fd92c805ea81fe5691e7b99e1d705476011 Mon Sep 17 00:00:00 2001 From: Hadley Wickham <h.wickham@gmail.com> Date: Tue, 12 Mar 2024 08:23:48 -0500 Subject: [PATCH 9/9] Drop unneccessary seed param doc --- R/build-reference.R | 2 -- man/build_reference.Rd | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/R/build-reference.R b/R/build-reference.R index bdb8d480a..ed127aab4 100644 --- a/R/build-reference.R +++ b/R/build-reference.R @@ -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()]. diff --git a/man/build_reference.Rd b/man/build_reference.Rd index 5fc668be2..2c0f25396 100644 --- a/man/build_reference.Rd +++ b/man/build_reference.Rd @@ -31,8 +31,8 @@ rapidly prototype. It is set to \code{FALSE} by \code{\link[=build_site]{build_s \item{run_dont_run}{Run examples that are surrounded in \\dontrun?} -\item{seed}{Seed used to initialize random number generation so that -examples are reproducible.} +\item{seed}{Seed used to initialize random number generation in order to +make article output reproducible. An integer scalar or \code{NULL} for no seed.} \item{override}{An optional named list used to temporarily override values in \verb{_pkgdown.yml}}