From 8a082a515a08dc46c5dae9ab36d6d4ffdcfebadc Mon Sep 17 00:00:00 2001 From: Tim Taylor Date: Tue, 20 Jul 2021 08:37:37 +0100 Subject: [PATCH 1/7] only check the scripts and report_sources folders with list_deps --- R/list_deps.R | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/R/list_deps.R b/R/list_deps.R index 28668d2..7cdd459 100644 --- a/R/list_deps.R +++ b/R/list_deps.R @@ -22,9 +22,14 @@ list_deps <- function(factory = ".", missing = FALSE, check_r = TRUE, exclude_re tmp <- suppressMessages(validate_factory(factory)) root <- tmp$root + config <- as.data.frame(read.dcf(file.path(root, "factory_config"))) + report_sources <- config$report_sources + root_report_sources <- file.path(root, report_sources) + root_scripts <- file.path(root, "scripts") + root_to_check <- c(root_report_sources, root_scripts) # Find dependencies in R files - r_files <- list.files(root, pattern = "\\.[Rr]$", recursive = TRUE, full.names = TRUE) + r_files <- list.files(root_to_check, pattern = "\\.[Rr]$", recursive = TRUE, full.names = TRUE) r_files_deps <- character(0) if (length(r_files) && check_r) { r_files_deps <- list_r_file_deps(r_files) @@ -34,7 +39,7 @@ list_deps <- function(factory = ".", missing = FALSE, check_r = TRUE, exclude_re # dependencies of code that is actually run are returned. op <- options(knitr.purl.inline = TRUE) on.exit(options(op)) - rmd_files <- list.files(root, pattern = "\\.[Rr]md$", recursive = TRUE, full.names = TRUE) + rmd_files <- list.files(root_to_check, pattern = "\\.[Rr]md$", recursive = TRUE, full.names = TRUE) if (exclude_readme) { readme <- list.files(pattern = "README\\.Rmd", recursive = TRUE, ignore.case = TRUE, full.names = TRUE) rmd_files <- rmd_files[!rmd_files %in% readme] From 8b1ca1d1f2bd54df8c96739d8c4bb446a1554be3 Mon Sep 17 00:00:00 2001 From: Tim Taylor Date: Tue, 20 Jul 2021 08:56:55 +0100 Subject: [PATCH 2/7] change binary argument to compare due to deprecation in testthat --- tests/testthat/test-compile_reports.R | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/testthat/test-compile_reports.R b/tests/testthat/test-compile_reports.R index 734144d..6d862cd 100644 --- a/tests/testthat/test-compile_reports.R +++ b/tests/testthat/test-compile_reports.R @@ -25,7 +25,7 @@ test_that("test parameteriesed report output", { md_file <- grep("\\.md", list_outputs(f), value = TRUE) md_file <- path(f, "outputs", md_file) - expect_snapshot_file(md_file, "param_report_check.md", binary = FALSE) + expect_snapshot_file(md_file, "param_report_check.md", compare = compare_file_text) }) test_that("test ignore_case works report output", { @@ -54,7 +54,7 @@ test_that("test ignore_case works report output", { md_file <- grep("\\.md", list_outputs(f), value = TRUE) md_file <- path(f, "outputs", md_file) - expect_snapshot_file(md_file, "param_report_check.md", binary = FALSE) + expect_snapshot_file(md_file, "param_report_check.md", compare = compare_file_text) # Should error if not case insensitive expect_error( @@ -98,7 +98,7 @@ test_that("test output folder gets recreated if not there", { md_file <- grep("\\.md", list_outputs(f), value = TRUE) md_file <- path(f, "outputs", md_file) - expect_snapshot_file(md_file, "outputs_deleted_param_report_check.md", binary = FALSE) + expect_snapshot_file(md_file, "outputs_deleted_param_report_check.md", compare = compare_file_text) }) @@ -127,7 +127,7 @@ test_that("parameteriesed report with missing param output but input", { md_file <- grep("\\.md", list_outputs(f), value = TRUE) md_file <- path(f, "outputs", md_file) - expect_snapshot_file(md_file, "missing_param_report_check.md", binary = FALSE) + expect_snapshot_file(md_file, "missing_param_report_check.md", compare = compare_file_text) }) test_that("non parameteriesed report with param input", { @@ -155,7 +155,7 @@ test_that("non parameteriesed report with param input", { md_file <- grep("\\.md", list_outputs(f), value = TRUE) md_file <- path(f, "outputs", md_file) - expect_snapshot_file(md_file, "nonparameterised_with_params.md", binary = FALSE) + expect_snapshot_file(md_file, "nonparameterised_with_params.md", compare = compare_file_text) }) test_that("parameteriesed report with missing param (but in environment)", { @@ -184,7 +184,7 @@ test_that("parameteriesed report with missing param (but in environment)", { md_file <- grep("\\.md", list_outputs(f), value = TRUE) md_file <- path(f, "outputs", md_file) - expect_snapshot_file(md_file, "missing_param_but_envir.md", binary = FALSE) + expect_snapshot_file(md_file, "missing_param_but_envir.md", compare = compare_file_text) }) From ff746b6ca30179a6bf2f30ae3c5ba61776d24a8e Mon Sep 17 00:00:00 2001 From: Tim Taylor Date: Tue, 20 Jul 2021 10:35:16 +0100 Subject: [PATCH 3/7] update list_deps documentation --- R/list_deps.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/list_deps.R b/R/list_deps.R index 7cdd459..fca2c6b 100644 --- a/R/list_deps.R +++ b/R/list_deps.R @@ -1,7 +1,7 @@ #' List dependencies of reports within a factory #' -#' This function can be used to list package dependencies based of the reports -#' and R scripts within the factory. +#' List package dependencies based on the reports and scripts within the +#' report_sources and scripts directories respectively. #' #' @inheritParams compile_reports #' @param missing A logical indicating if only missing dependencies should be From 1788cfdce0fdaba52cea443068d76a15bd954bf0 Mon Sep 17 00:00:00 2001 From: Tim Taylor Date: Tue, 20 Jul 2021 10:35:38 +0100 Subject: [PATCH 4/7] update NEWS for list_deps --- NEWS.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/NEWS.md b/NEWS.md index a672394..2d5f2ac 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,8 @@ # reportfactory (development version) +* `list_deps()` now only checks the `report_sources` and `scripts` folders for + package dependencies. + # reportfactory 0.3.1 * Fixed `list_deps()`, which was broken due to major changes in the From 76d1f22dce0e3563d2f0119bf9eb7987484dc272 Mon Sep 17 00:00:00 2001 From: Tim Taylor Date: Fri, 30 Jul 2021 15:05:00 +0100 Subject: [PATCH 5/7] tidying --- R/internals.R | 6 ++++++ R/list_deps.R | 31 +++++++++++++++++++++++++++---- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/R/internals.R b/R/internals.R index b2e1021..4d7c0ad 100644 --- a/R/internals.R +++ b/R/internals.R @@ -37,6 +37,8 @@ factory_root <- function(directory = ".") { root } +# ------------------------------------------------------------------------- + #' check whether a vector is "integer-like" to a given precision #' #' @param x vector to check @@ -47,6 +49,8 @@ is.wholenumber <- function(x, tol = .Machine$double.eps^0.5) { all(abs(x - round(x)) < tol) } +# ------------------------------------------------------------------------- + #' copy a file from the skeleton directory #' #' @param file name of the file you want to copy @@ -58,6 +62,7 @@ copy_skeleton_file <- function(file, dest) { file.copy(f, dest) } +# ------------------------------------------------------------------------- #' Change part of the front yaml matter from an Rmarkdown file #' @@ -103,6 +108,7 @@ change_yaml_matter <- function(input_file, ..., output_file) { } } +# ------------------------------------------------------------------------- #' Return rows of one data.frame not in another #' diff --git a/R/list_deps.R b/R/list_deps.R index fca2c6b..31af1b8 100644 --- a/R/list_deps.R +++ b/R/list_deps.R @@ -29,7 +29,13 @@ list_deps <- function(factory = ".", missing = FALSE, check_r = TRUE, exclude_re root_to_check <- c(root_report_sources, root_scripts) # Find dependencies in R files - r_files <- list.files(root_to_check, pattern = "\\.[Rr]$", recursive = TRUE, full.names = TRUE) + r_files <- list.files( + root_to_check, + pattern = "\\.[Rr]$", + recursive = TRUE, + full.names = TRUE + ) + r_files_deps <- character(0) if (length(r_files) && check_r) { r_files_deps <- list_r_file_deps(r_files) @@ -39,9 +45,19 @@ list_deps <- function(factory = ".", missing = FALSE, check_r = TRUE, exclude_re # dependencies of code that is actually run are returned. op <- options(knitr.purl.inline = TRUE) on.exit(options(op)) - rmd_files <- list.files(root_to_check, pattern = "\\.[Rr]md$", recursive = TRUE, full.names = TRUE) + rmd_files <- list.files( + root_to_check, + pattern = "\\.[Rr]md$", + recursive = TRUE, + full.names = TRUE + ) if (exclude_readme) { - readme <- list.files(pattern = "README\\.Rmd", recursive = TRUE, ignore.case = TRUE, full.names = TRUE) + readme <- list.files( + pattern = "README\\.Rmd", + recursive = TRUE, + ignore.case = TRUE, + full.names = TRUE + ) rmd_files <- rmd_files[!rmd_files %in% readme] } @@ -56,7 +72,12 @@ list_deps <- function(factory = ".", missing = FALSE, check_r = TRUE, exclude_re fe <- file(fefil, "w") sink(fe, type = "message") mapply( - function(x,y) try(knitr::purl(input = x, output = y, quiet = TRUE, documentation = 0), silent = TRUE), + function(x,y) { + try( + knitr::purl(input = x, output = y, quiet = TRUE, documentation = 0), + silent = TRUE + ) + } , rmd_files, fd ) @@ -74,6 +95,8 @@ list_deps <- function(factory = ".", missing = FALSE, check_r = TRUE, exclude_re deps } +# ------------------------------------------------------------------------- + list_r_file_deps <- function(filepaths) { dat <- vapply( From 4c3ff1ea43591bf233d4668b3b3eb8a1773db010 Mon Sep 17 00:00:00 2001 From: Tim Taylor Date: Mon, 9 Aug 2021 10:05:50 +0100 Subject: [PATCH 6/7] Default to simple readLines parsing or reports and scripts --- R/list_deps.R | 93 ++++++++++++++++++------------- inst/skeletons/example_report.Rmd | 4 ++ man/list_deps.Rd | 13 ++++- tests/testthat/test-list_deps.R | 38 ++++++++++--- 4 files changed, 98 insertions(+), 50 deletions(-) diff --git a/R/list_deps.R b/R/list_deps.R index 31af1b8..d6fc9cb 100644 --- a/R/list_deps.R +++ b/R/list_deps.R @@ -11,6 +11,11 @@ #' checked. Note that this will error if the script cannot be parsed. #' @param exclude_readme If TRUE (default) README files will not be checked for #' dependencies. +#' @param parse_first If `TRUE` code will first be parsed for validity and +#' unevaluated Rmd chunks will not be checked for dependencies. The default +#' value is `FALSE` and, in this case, files will simply be checked line by +#' line for calls to `library`, `require` or use of double, `::`, and triple, +#' `:::` function calls. #' #' @note This function requires that any R scripts present in the factory are #' valid syntax else the function will error. @@ -18,7 +23,8 @@ #' @return A character vector of package dependencies. #' #' @export -list_deps <- function(factory = ".", missing = FALSE, check_r = TRUE, exclude_readme = TRUE) { +list_deps <- function(factory = ".", missing = FALSE, check_r = TRUE, + exclude_readme = TRUE, parse_first = FALSE) { tmp <- suppressMessages(validate_factory(factory)) root <- tmp$root @@ -28,7 +34,7 @@ list_deps <- function(factory = ".", missing = FALSE, check_r = TRUE, exclude_re root_scripts <- file.path(root, "scripts") root_to_check <- c(root_report_sources, root_scripts) - # Find dependencies in R files + # List of R files r_files <- list.files( root_to_check, pattern = "\\.[Rr]$", @@ -36,18 +42,9 @@ list_deps <- function(factory = ".", missing = FALSE, check_r = TRUE, exclude_re full.names = TRUE ) - r_files_deps <- character(0) - if (length(r_files) && check_r) { - r_files_deps <- list_r_file_deps(r_files) - } - - # Find dependencies in Rmd files. We knit the files first to ensure only - # dependencies of code that is actually run are returned. - op <- options(knitr.purl.inline = TRUE) - on.exit(options(op)) + # List of Rmd files rmd_files <- list.files( - root_to_check, - pattern = "\\.[Rr]md$", + root_to_check, pattern = "\\.[Rr]md$", recursive = TRUE, full.names = TRUE ) @@ -61,31 +58,45 @@ list_deps <- function(factory = ".", missing = FALSE, check_r = TRUE, exclude_re rmd_files <- rmd_files[!rmd_files %in% readme] } + # Find R file dependencies + r_files_deps <- character(0) + if (length(r_files) && check_r) { + r_files_deps <- list_r_file_deps(r_files, parse = parse_first) + } + + # Find Rmd file dependencies + op <- options(knitr.purl.inline = TRUE) + on.exit(options(op)) rmd_files_deps <- character(0) if (length(rmd_files)) { - d <- tempdir() - fd <- sub(pattern = "(.*)\\..*$", replacement = "\\1", basename(rmd_files)) - fd <- vapply(fd, function(x) file.path(d, x), character(1)) - on.exit(unlink(fd), add = TRUE) - fefil <- tempfile() - on.exit(unlink(fefil), add = TRUE) - fe <- file(fefil, "w") - sink(fe, type = "message") - mapply( - function(x,y) { - try( - knitr::purl(input = x, output = y, quiet = TRUE, documentation = 0), - silent = TRUE - ) - } , - rmd_files, - fd - ) - sink(type = "message") - close(fe) - rmd_files_deps <- c("rmarkdown", list_r_file_deps(fd)) + if (parse_first) { + d <- tempdir() + fd <- sub(pattern = "(.*)\\..*$", replacement = "\\1", basename(rmd_files)) + fd <- vapply(fd, function(x) file.path(d, x), character(1)) + on.exit(unlink(fd), add = TRUE) + fefil <- tempfile() + on.exit(unlink(fefil), add = TRUE) + fe <- file(fefil, "w") + sink(fe, type = "message") + mapply( + function(x,y) { + try( + knitr::purl(input = x, output = y, quiet = TRUE, documentation = 0), + silent = TRUE + ) + } , + rmd_files, + fd + ) + sink(type = "message") + close(fe) + rmd_files_deps <- c("rmarkdown", list_r_file_deps(fd, parse = TRUE)) + } else { + rmd_files_deps <- c("rmarkdown", list_r_file_deps(rmd_files, parse = FALSE)) + } } + # return unique dependencies deps <- unique(c(r_files_deps, rmd_files_deps)) if (missing) { installed <- basename(find.package(deps)) @@ -97,13 +108,15 @@ list_deps <- function(factory = ".", missing = FALSE, check_r = TRUE, exclude_re # ------------------------------------------------------------------------- -list_r_file_deps <- function(filepaths) { +list_r_file_deps <- function(filepaths, parse = FALSE) { - dat <- vapply( - filepaths, - function(x) paste(as.character(parse(x)), collapse = "\n"), - character(1) - ) + if (parse) { + f <- function(x) paste(as.character(parse(x)), collapse = "\n") + } else { + f <- function(x) paste(readLines(x), collapse = "\n") + } + + dat <- vapply(filepaths, f, character(1)) colon_string <- r"---{([a-zA-Z][\w.]*)(?=:{2,3})}---" colon_greg <- gregexpr(colon_string, dat, perl = TRUE) diff --git a/inst/skeletons/example_report.Rmd b/inst/skeletons/example_report.Rmd index f10636b..5f4fff8 100644 --- a/inst/skeletons/example_report.Rmd +++ b/inst/skeletons/example_report.Rmd @@ -12,6 +12,10 @@ output: knitr::opts_chunk$set(echo = TRUE, collapse = TRUE, fig.width = 8, fig.height = 6, dpi = 70) ``` +```{r, eval = FALSE} +base::mean(c(1,2)) +``` + This is an example report to illustrate how `reportfactory` can be used. It loads the `fs` library purely for demonstration purposes. diff --git a/man/list_deps.Rd b/man/list_deps.Rd index 75b0389..8da0c4f 100644 --- a/man/list_deps.Rd +++ b/man/list_deps.Rd @@ -8,7 +8,8 @@ list_deps( factory = ".", missing = FALSE, check_r = TRUE, - exclude_readme = TRUE + exclude_readme = TRUE, + parse_first = FALSE ) } \arguments{ @@ -24,13 +25,19 @@ checked. Note that this will error if the script cannot be parsed.} \item{exclude_readme}{If TRUE (default) README files will not be checked for dependencies.} + +\item{parse_first}{If \code{TRUE} code will first be parsed for validity and +unevaluated Rmd chunks will not be checked for dependencies. The default +value is \code{FALSE} and, in this case, files will simply be checked line by +line for calls to \code{library}, \code{require} or use of double, \code{::}, and triple, +\code{:::} function calls.} } \value{ A character vector of package dependencies. } \description{ -This function can be used to list package dependencies based of the reports -and R scripts within the factory. +List package dependencies based on the reports and scripts within the +report_sources and scripts directories respectively. } \note{ This function requires that any R scripts present in the factory are diff --git a/tests/testthat/test-list_deps.R b/tests/testthat/test-list_deps.R index 8fa3066..5179842 100644 --- a/tests/testthat/test-list_deps.R +++ b/tests/testthat/test-list_deps.R @@ -1,6 +1,6 @@ library(fs) -test_that("list_deps works", { +test_that("non parsing list_deps works", { f <- new_factory(path = path_temp(), move_in = FALSE) on.exit(dir_delete(f)) @@ -9,13 +9,37 @@ test_that("list_deps works", { file_copy(path("test_reports", "package_calls.Rmd"), path(f, "report_sources")) file_copy(path("test_reports", "example.R"), path(f, "report_sources")) - expected_deps_package_calls <- c("purrr", "readxl", "fs", "rmarkdown", "utils", "yaml", "callr", "rprojroot") + # non parsed expected_deps + expected_deps_package_calls_rmd <- c("purrr", "readxl", "fs", "rmarkdown") + expected_deps_example_r <- c("fs", "yaml", "callr", "utils", "rprojroot") + expected_deps_example_report <- c("base", "knitr", "fs") + expected_deps <- c(expected_deps_package_calls_rmd, + expected_deps_example_r, + expected_deps_example_report) - expected_deps_example_report <- c("knitr", "fs", "rmarkdown") - expected_deps_readme <- "rmarkdown" - expected_deps <- c(expected_deps_package_calls, - expected_deps_example_report, - expected_deps_readme) deps <- list_deps(f) expect_equal(sort(deps), sort(unique(expected_deps))) }) + + +test_that("parsing list_deps works", { + + f <- new_factory(path = path_temp(), move_in = FALSE) + on.exit(dir_delete(f)) + + # copy test reports over (as this has inline code) + file_copy(path("test_reports", "package_calls.Rmd"), path(f, "report_sources")) + file_copy(path("test_reports", "example.R"), path(f, "report_sources")) + + # non parsed expected_deps + expected_deps_package_calls_rmd <- c("purrr", "readxl", "fs", "rmarkdown") + expected_deps_example_r <- c("fs", "yaml", "callr", "utils", "rprojroot") + expected_deps_example_report <- c("knitr", "fs") + expected_deps <- c(expected_deps_package_calls_rmd, + expected_deps_example_r, + expected_deps_example_report) + + deps <- list_deps(f, parse_first = TRUE) + expect_equal(sort(deps), sort(unique(expected_deps))) +}) + From 6262868620d928d05e91c82e1cae0cb509434a83 Mon Sep 17 00:00:00 2001 From: Tim Taylor Date: Mon, 9 Aug 2021 12:58:16 +0100 Subject: [PATCH 7/7] CRAN prep --- DESCRIPTION | 2 +- NEWS.md | 2 +- cran-comments.md | 6 ++---- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index fbcd83c..dda3bd4 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: reportfactory Title: Lightweight Infrastructure for Handling Multiple R Markdown Documents -Version: 0.3.1.9000 +Version: 0.4.0 Authors@R: c( person("Thibaut", "Jombart", role = "aut", email = "thibautjombart@gmail.com"), person("Amy", "Gimma", role = "ctb", email = "amyg225@gmail.com"), diff --git a/NEWS.md b/NEWS.md index 9a993f5..f3783fb 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,4 +1,4 @@ -# reportfactory (development version) +# reportfactory 0.4.0 * `list_deps()` now only checks the `report_sources` and `scripts` folders for package dependencies. diff --git a/cran-comments.md b/cran-comments.md index 27aff2b..83282b9 100644 --- a/cran-comments.md +++ b/cran-comments.md @@ -1,8 +1,6 @@ -# Reason for update -* Fixes due to breaking changes in the upstream checkpoint package. - # Tested on -* Fedora 34, R Under development (unstable) (2021-06-16 r80504) +* Fedora 34, R Under development (unstable) (2021-08-09 r80724) +* Winbuilder, R Under development (unstable) (2021-08-05 r80717) ## R CMD check results for above environments Status: OK