diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml index 0f5ea8220..82c08c3e9 100644 --- a/.github/workflows/R-CMD-check.yaml +++ b/.github/workflows/R-CMD-check.yaml @@ -30,6 +30,7 @@ jobs: env: R_REMOTES_NO_ERRORS_FROM_WARNINGS: true + R_STYLER_FUTURE_NO_OVERRIDE: true RSPM: ${{ matrix.config.rspm }} GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} _R_CHECK_FORCE_SUGGESTS_: false diff --git a/.github/workflows/benchmarking.yaml b/.github/workflows/benchmarking.yaml index 457ba02ed..2672141b5 100644 --- a/.github/workflows/benchmarking.yaml +++ b/.github/workflows/benchmarking.yaml @@ -11,6 +11,7 @@ jobs: env: R_REMOTES_NO_ERRORS_FROM_WARNINGS: true + R_STYLER_FUTURE_NO_OVERRIDE: true RSPM: ${{ matrix.config.rspm }} GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} steps: diff --git a/.github/workflows/test-coverage.yaml b/.github/workflows/test-coverage.yaml index 3058d037b..3397bffb6 100644 --- a/.github/workflows/test-coverage.yaml +++ b/.github/workflows/test-coverage.yaml @@ -13,6 +13,7 @@ jobs: runs-on: macOS-latest env: GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} + R_STYLER_FUTURE_NO_OVERRIDE: true steps: - uses: actions/checkout@v2 diff --git a/DESCRIPTION b/DESCRIPTION index f2447f4c3..69554b913 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -33,6 +33,8 @@ Suggests: data.tree (>= 0.1.6), digest, dplyr, + furrr, + future, here, knitr, prettycode, @@ -56,6 +58,7 @@ Collate: 'detect-alignment.R' 'environments.R' 'expr-is.R' + 'future.R' 'indent.R' 'initialize.R' 'io.R' diff --git a/NAMESPACE b/NAMESPACE index 7ab16f529..348cfac29 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -40,6 +40,7 @@ importFrom(purrr,when) importFrom(rlang,abort) importFrom(rlang,is_empty) importFrom(rlang,is_installed) +importFrom(rlang,local_options) importFrom(rlang,seq2) importFrom(rlang,warn) importFrom(rlang,with_handlers) diff --git a/R/future.R b/R/future.R new file mode 100644 index 000000000..218dd269a --- /dev/null +++ b/R/future.R @@ -0,0 +1,20 @@ +#' Parallelization of styling +#' +#' [style_file()], [style_pkg()] and [style_dir()] leverage the `{future}` +#' framework for parallelization. To make use of parallel processing, you need +#' to have the package `{furrr}` installed. In that case, the strategy +#' [future::multiprocess()] is set temporarily during styling (only if there is +#' more than two files to style, because of start-up costs of parallelization). +#' You can prevent this temporary change in the future strategy by setting the +#' environment variable `R_STYLER_FUTURE_NO_OVERRIDE` to `TRUE` (case ignored). In +#' that case, the future strategy set before calling styler will be used, if +#' you have not set any, the future `{framework}` falls back to +#' [future::sequential()]. +#' +#' @section Life-cycle: +#' The parallelization feature is experimental, in particular how its governed +#' with `R_STYLER_FUTURE_NO_OVERRIDE` and how progress bars are displayed with +#' `{furrr}`. In the future, most likely the `{progressr}` package will be used +#' to handle progress bars. +#' @name styler_future +NULL diff --git a/R/transform-files.R b/R/transform-files.R index f887391e9..78e29df15 100644 --- a/R/transform-files.R +++ b/R/transform-files.R @@ -11,17 +11,44 @@ #' styling whether or not it was actually changed (or would be changed when #' `dry` is not "off"). #' @keywords internal +#' @importFrom rlang local_options transform_files <- function(files, transformers, include_roxygen_examples, base_indention, dry) { transformer <- make_transformer(transformers, include_roxygen_examples, base_indention) max_char <- min(max(nchar(files), 0), getOption("width")) len_files <- length(files) if (len_files > 0L && !getOption("styler.quiet", FALSE)) { - cat("Styling ", len_files, " files:\n") + cat("Styling ", len_files, " files:\n") # TODO should only have one space between words + } + + future_strategy_no_override <- tolower(Sys.getenv("R_STYLER_FUTURE_NO_OVERRIDE", "FALSE")) == "true" + if (rlang::is_installed("furrr")) { + # only parallelise when furrr is installed + if (!future_strategy_no_override && length(files) > 2) { + # only override when env not set and more than two files. + local_options(future.supportsMulticore.unstable = "quiet") + oplan <- future::plan("multisession") # not sure we should use + + # withr::defer() gives C stack overflow on macOS, on.exit not. + on.exit(future::plan(oplan), add = TRUE, after = FALSE) + } + changed <- furrr::future_map_lgl(files, transform_file, + fun = transformer, max_char_path = max_char, dry = dry, + .progress = TRUE + ) + } else { + if (future_strategy_no_override) { + rlang::warn(paste( + "Environment variable `R_STYLER_FUTURE_NO_OVERRIDE` is not respected", + "because package {furrr} is not installed. Falling back to ", + "`purrr::map_lgl()` to iterate over files to style instead of", + "`furrr::future_map_lgl()`." + )) + } + changed <- map_lgl(files, transform_file, + fun = transformer, max_char_path = max_char, dry = dry + ) } - changed <- map_lgl(files, transform_file, - fun = transformer, max_char_path = max_char, dry = dry - ) communicate_summary(changed, max_char) communicate_warning(changed, transformers) new_tibble(list(file = files, changed = changed), nrow = len_files) diff --git a/inst/WORDLIST b/inst/WORDLIST index 9d8b5fff3..ca0ce9786 100644 --- a/inst/WORDLIST +++ b/inst/WORDLIST @@ -58,6 +58,7 @@ filetype forcond formatter funct +furrr gcc getChecksum getOption @@ -100,6 +101,7 @@ macOS magrittr md Müller +multiprocess mutli na navbar @@ -111,6 +113,8 @@ ourself packagemanager packrat pandoc +parallelization +Parallelization params paren parsable @@ -124,6 +128,7 @@ pre precommit prefill prettycode +progressr PRs purrr rcmdcheck diff --git a/man/styler_future.Rd b/man/styler_future.Rd new file mode 100644 index 000000000..f9bbaaf2c --- /dev/null +++ b/man/styler_future.Rd @@ -0,0 +1,25 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/future.R +\name{styler_future} +\alias{styler_future} +\title{Parallelization of styling} +\description{ +\code{\link[=style_file]{style_file()}}, \code{\link[=style_pkg]{style_pkg()}} and \code{\link[=style_dir]{style_dir()}} leverage the \code{{future}} +framework for parallelization. To make use of parallel processing, you need +to have the package \code{{furrr}} installed. In that case, the strategy +\code{\link[future:multiprocess]{future::multiprocess()}} is set temporarily during styling (only if there is +more than two files to style, because of start-up costs of parallelization). +You can prevent this temporary change in the future strategy by setting the +environment variable \code{R_STYLER_FUTURE_NO_OVERRIDE} to \code{TRUE} (case ignored). In +that case, the future strategy set before calling styler will be used, if +you have not set any, the future \code{{framework}} falls back to +\code{\link[future:sequential]{future::sequential()}}. +} +\section{Life-cycle}{ + +The parallelization feature is experimental, in particular how its governed +with \code{R_STYLER_FUTURE_NO_OVERRIDE} and how progress bars are displayed with +\code{{furrr}}. In the future, most likely the \code{{progressr}} package will be used +to handle progress bars. +} + diff --git a/tests/testthat/test-future.R b/tests/testthat/test-future.R new file mode 100644 index 000000000..db70307b6 --- /dev/null +++ b/tests/testthat/test-future.R @@ -0,0 +1,53 @@ +path_rel <- c( + "dirty-sample-with-scope-tokens.R", + "dirty-sample-with-scope-spaces.R", + "clean-sample-with-scope-tokens.R" +) + +path_dir <- file.path( + "public-api", "xyzdir-dirty", path_rel +) + +test_that("can style in parallel when no strategy is set and overriding is deactivated", { + withr::local_envvar("R_STYLER_FUTURE_NO_OVERRIDE" = "TRUE") + expect_match( + catch_style_file_output(path_dir), + 'Styling +3', + all = FALSE + ) +}) + +test_that("can style in parallel when no strategy is set and overriding is not deactivated", { + withr::local_envvar("R_STYLER_FUTURE_NO_OVERRIDE" = "FALSE") + expect_match( + catch_style_file_output(path_dir), + 'Styling +3', + all = FALSE + ) +}) + +test_that("can style in parallel when no strategy is set", { + + expect_match( + catch_style_file_output(path_dir), + 'Styling +3', + all = FALSE + ) +}) + +test_that("can style in parallel when strategy is set", { + future::plan('sequential') + expect_match( + catch_style_file_output(path_dir), + 'Styling +3', + all = FALSE + ) +}) + +test_that("with fewer than two files, the plan is not modified", { + expect_match( + catch_style_file_output(path_dir[1]), + 'Styling +1', + all = FALSE + ) +})