diff --git a/.Rbuildignore b/.Rbuildignore index 6f1bea5..ddca154 100644 --- a/.Rbuildignore +++ b/.Rbuildignore @@ -6,7 +6,9 @@ ^codecov\.yml$ ^data-raw$ ^\.vscode$ +^bin$ ^.lintr$ ^CHANGELOG\.md$ +^NOTES\.md$ ^cran-comments\.md$ ^CRAN-SUBMISSION$ diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml index 04a4415..a35e3cd 100644 --- a/.github/workflows/R-CMD-check.yaml +++ b/.github/workflows/R-CMD-check.yaml @@ -1,5 +1,10 @@ on: push: + paths-ignore: + - "*.md" + - "*.Rmd" + - ".vscode/**" + - "bin/**" branches: - main - release/* @@ -9,6 +14,11 @@ on: - develop - release/* - hotfix/* + paths-ignore: + - "*.md" + - "*.Rmd" + - ".vscode/**" + - "bin/**" workflow_dispatch: name: R-CMD-check diff --git a/.github/workflows/test-coverage.yaml b/.github/workflows/test-coverage.yaml index 1675310..5d2cbbc 100644 --- a/.github/workflows/test-coverage.yaml +++ b/.github/workflows/test-coverage.yaml @@ -1,8 +1,18 @@ on: push: branches: [main] + paths-ignore: + - "*.md" + - "*.Rmd" + - ".vscode/**" + - "bin/**" pull_request: branches: [main] + paths-ignore: + - "*.md" + - "*.Rmd" + - ".vscode/**" + - "bin/**" name: test-coverage diff --git a/.github/workflows/test-non-interactive.yaml b/.github/workflows/test-non-interactive.yaml new file mode 100644 index 0000000..9e9b7de --- /dev/null +++ b/.github/workflows/test-non-interactive.yaml @@ -0,0 +1,75 @@ +on: + push: + branches: + - main + - release/* + - hotfix/* + paths-ignore: + - "*.md" + - "*.Rmd" + - ".vscode/**" + pull_request: + branches: + - develop + - release/* + - hotfix/* + paths-ignore: + - "*.md" + - "*.Rmd" + - ".vscode/**" + workflow_dispatch: + +name: Test non-interactive + +jobs: + test: + runs-on: ${{ matrix.config.os }} + + name: ${{ matrix.config.os }} (${{ matrix.config.r }}) + + strategy: + fail-fast: false + matrix: + config: + - { os: macOS-latest, r: "release" } + - { os: windows-latest, r: "release" } + # - { os: windows-latest, r: "3.6" } the cli::rule dashes get messed up + - { os: ubuntu-latest, r: "devel", http-user-agent: "release" } + - { os: ubuntu-latest, r: "release" } + - { os: ubuntu-latest, r: "oldrel-1" } + + env: + GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} + R_KEEP_PKG_SOURCE: yes + + steps: + # https://github.com/actions/checkout + - uses: actions/checkout@v3 + + # https://github.com/r-lib/actions/tree/v2/setup-pandoc + - uses: r-lib/actions/setup-pandoc@v2 + + # https://github.com/r-lib/actions/tree/v2/setup-r + - uses: r-lib/actions/setup-r@v2 + with: + r-version: ${{ matrix.config.r }} + http-user-agent: ${{ matrix.config.http-user-agent }} + use-public-rspm: true + + # https://github.com/r-lib/actions/tree/v2/setup-r-dependencies + - uses: r-lib/actions/setup-r-dependencies@v2 + with: + extra-packages: any::devtools + needs: check + + - name: Install the package + run: Rscript -e 'devtools::install()' + + - name: Test loading in non-interactive mode (normal) + run: Rscript bin/load-non-interactive.r + + - name: Test loading in non-interactive mode (quiet) + run: Rscript bin/load-non-interactive-quiet.r + + - name: Test loading in non-interactive mode (quickstart) + run: Rscript bin/load-non-interactive-quickstart.r diff --git a/.gitignore b/.gitignore index 11d486b..f0a1b87 100644 --- a/.gitignore +++ b/.gitignore @@ -2,5 +2,6 @@ .Rhistory .RData .DS_Store +NOTES.md README.html CRAN-SUBMISSION diff --git a/DESCRIPTION b/DESCRIPTION index 2a47b88..c4f8bb5 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: coursekata Title: Packages and Functions for 'CourseKata' Courses -Version: 0.14.0 -Date: 2023-10-11 +Version: 0.14.1 +Date: 2023-10-26 Authors@R: c( person("Adam", "Blake", , "adamblake@g.ucla.edu", role = c("cre", "aut"), comment = c(ORCID = "0000-0001-7881-8652")), @@ -35,8 +35,7 @@ Imports: rlang (>= 1.0.2), supernova (>= 2.5.1), vctrs (>= 0.4.1), - viridisLite, - yesno (>= 0.1.2) + viridisLite Suggests: fivethirtyeight (>= 0.6.2), lubridate (>= 1.8.0), diff --git a/NEWS.md b/NEWS.md index e36dc1d..008826f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,11 @@ # `coursekata` change log +## coursekata 0.14.1 + +- Reduce calls to `pak::pkg_status()` to improve startup time +- Address issue where `require(lib.loc = ...)` was sometimes being passed `NA` +- Appropriately skip actions that require the user when running in non-interactive mode (and add related tests) + ## coursekata 0.14.0 - Remove deprecated `gf_model_old()` function diff --git a/R/coursekata_attach.R b/R/coursekata_attach.R index 7b25948..8eb11be 100644 --- a/R/coursekata_attach.R +++ b/R/coursekata_attach.R @@ -1,5 +1,6 @@ #' Attach the CourseKata course packages #' +#' @param do_not_ask Prevent asking the user to install missing packages (they are skipped). #' @param quietly Whether to suppress messages. #' #' @return A named logical vector indicating which packages were attached. @@ -7,9 +8,15 @@ #' @export #' @examples #' coursekata_attach() -coursekata_attach <- function(quietly = FALSE) { - to_attach <- coursekata_detached() - invisible(pkg_require(to_attach, quietly = quietly)) +coursekata_attach <- function(do_not_ask = FALSE, quietly = FALSE) { + !do_not_ask && pkg_check_installed(coursekata_pkgs) + detached <- coursekata_detached() + installed <- coursekata_pkgs[pkg_is_installed(coursekata_pkgs)] + attached <- pkg_require(detached[detached %in% installed], quietly = quietly) + + result <- rep_named(detached, FALSE) + result[names(attached)] <- TRUE + invisible(result) } @@ -23,7 +30,9 @@ coursekata_attach <- function(quietly = FALSE) { #' #' @noRd coursekata_attach_message <- function(pkgs) { - if (length(pkgs) == 0) return(NULL) + if (length(pkgs) == 0) { + return(NULL) + } info <- coursekata_packages() version <- ifelse(is.na(info$version), "", info$version) @@ -42,3 +51,13 @@ coursekata_attach_message <- function(pkgs) { sep = "\n" ) } + + +#' List all currently NOT attached CourseKata course packages +#' +#' @return A character vector of the course packages that are not attached. +#' +#' @noRd +coursekata_detached <- function() { + coursekata_pkgs[!pkg_is_attached(coursekata_pkgs)] +} diff --git a/R/coursekata_install.R b/R/coursekata_install.R index cd23ae6..63d7917 100644 --- a/R/coursekata_install.R +++ b/R/coursekata_install.R @@ -18,7 +18,7 @@ coursekata_install <- function(...) { } cli::cat_line() - pkg_install(behind$package, ...) + pak::pkg_install(pkg_fix_remote_names(behind$package), ...) invisible(coursekata_packages(TRUE)) } diff --git a/R/coursekata_packages.R b/R/coursekata_packages.R index 5080f37..b85bc38 100644 --- a/R/coursekata_packages.R +++ b/R/coursekata_packages.R @@ -21,7 +21,7 @@ coursekata_packages <- function(check_remote_version = FALSE) { statuses <- pak::pkg_status(pkgs) info <- data.frame( package = pkgs, - installed = pkg_is_installed(pkgs, statuses), + installed = pkg_is_installed(pkgs), attached = pkg_is_attached(pkgs), version = pkg_version(pkgs, statuses), stringsAsFactors = FALSE @@ -34,23 +34,3 @@ coursekata_packages <- function(check_remote_version = FALSE) { info } - - -#' List all currently attached CourseKata course packages -#' -#' @return A character vector of the course packages that have been attached. -#' -#' @noRd -coursekata_attached <- function() { - coursekata_pkgs[pkg_is_attached(coursekata_pkgs)] -} - - -#' List all currently NOT attached CourseKata course packages -#' -#' @return A character vector of the course packages that are not attached. -#' -#' @noRd -coursekata_detached <- function() { - coursekata_pkgs[!pkg_is_attached(coursekata_pkgs)] -} diff --git a/R/utils-pkg.R b/R/utils-pkg.R index f754aad..87524e6 100644 --- a/R/utils-pkg.R +++ b/R/utils-pkg.R @@ -10,36 +10,15 @@ pkg_is_attached <- function(pkgs) { } -#' Check if packages are installed -#' -#' Note: this function differs from [`rlang::is_installed()`] in two regards: it is quieter and will -#' show no messages, and it returns a vector indicating which packages are installed or not (rather -#' than a single Boolean value regarding the packages as a set). +#' Check if packages are installed. #' #' @param pkgs Character vector of the names of the packages to check. -#' @param statuses The output of [`pak::pkg_status()`] (computed if not supplied). #' #' @return Named logical vector indicating whether the packages are installed. #' #' @noRd -pkg_is_installed <- function(pkgs, statuses = NULL) { - statuses <- if (is.null(statuses)) pak::pkg_status(pkgs) else statuses - checker <- function(pkg) pkg %in% statuses$package - vapply(pkgs, checker, logical(1)) -} - - -#' Determine which libraries packages were loaded from -#' -#' @param pkgs A character vector of packages to check. -#' @param statuses The output of [`pak::pkg_status()`] (computed if not supplied). -#' -#' @return A character vector of library directory paths the packages were loaded from, the default -#' location if the package is not loaded but is installed, or NA if the package is not installed. -#' -#' @noRd -pkg_library_location <- function(pkgs, statuses = NULL) { - possibly_pkg_status(pkgs, "library", statuses = statuses) +pkg_is_installed <- function(pkgs) { + vapply(pkgs, rlang::is_installed, logical(1)) } @@ -97,79 +76,55 @@ pkg_remote_version <- function(pkgs) { #' place it was already loaded from. #' #' @param pkgs A character vector of packages to load. -#' @param do_not_ask Prevent asking the user to install missing packages (they are skipped). #' @param quietly Prevent package startup messages. #' #' @return A named logical indicating whether each package was loaded. #' #' @noRd -pkg_require <- function(pkgs, do_not_ask = FALSE, quietly = TRUE) { - pkg_load <- if (quietly) { - function(pkg) { - suppressPackageStartupMessages(suppressWarnings(require( - pkg, - lib.loc = if (quickstart()) NULL else pkg_library_location(pkg), - character.only = TRUE, - warn.conflicts = FALSE, - quietly = TRUE - ))) - } - } else { - function(pkg) { - require( - pkg, - lib.loc = if (quickstart()) NULL else pkg_library_location(pkg), - character.only = TRUE, - ) - } +pkg_require <- function(pkgs, quietly = TRUE) { + quiet_wrap <- function(fn) { + function(...) suppressPackageStartupMessages(suppressWarnings(fn(...))) } - vapply(pkgs, function(pkg) { - loaded <- pkg_load(pkg) - if (!loaded && !do_not_ask && ask_to_install(pkg)) { - pkg_install(pkg) - pkg_load(pkg) - } else { - loaded - } - }, logical(1)) + pkg_load <- function(pkg) { + lib_loc <- if (pkg %in% loadedNamespaces()) dirname(getNamespaceInfo(pkg, "path")) else NULL + require(pkg, lib.loc = lib_loc, character.only = TRUE, quietly = quietly) + } + + loader <- if (quietly) quiet_wrap(pkg_load) else pkg_load + vapply(pkgs, loader, logical(1)) } -#' Ask the user if they want to install packages +#' Ask the user if they want to install packages then install them. #' #' @param pkgs A character vector of packages to ask about. #' -#' @return A logical indicating whether the user answered yes or no. +#' @return Whether the packages were installed or not. #' #' @noRd -ask_to_install <- function(pkgs) { +pkg_check_installed <- function(pkgs) { if (!interactive()) { return(FALSE) } - line <- function(x = "") { - sprintf("%s\n", x) - } - yesno::yesno(cli::col_red(paste0( - line(), - line("The following packages could not be found:"), - line(paste0(" - ", pkgs, "\n", collapse = "")), - "Install missing packages?" - ))) + withRestarts( + is.null(rlang::check_installed(pkgs, action = function(pkgs, ...) { + pkgs <- pkg_fix_remote_names(pkgs) + pak::pkg_install(pkgs, ask = FALSE, ...) + })), + abort = function(e) FALSE + ) } - -#' Install packages using appropriate repositories. +#' Replace package names with their remote names (i.e. qualify with repo name). #' -#' @param pkgs A character vector of the packages to install. -#' @param ... Arguments passed on to [`pak::pkg_install()`]. +#' @param pkgs A character vector of package names. #' -#' @return The output of [`pak::pkg_install()`] if any packages were installed, else `NULL`. +#' @return A character vector of package names with remote names. #' #' @noRd -pkg_install <- function(pkgs, ...) { - is_538 <- pkgs %in% "fivethirtyeightdata" - if (any(is_538)) pak::pkg_install("fivethirtyeightdata/fivethirtyeightdata", ...) - if (any(!is_538)) pak::pkg_install(pkgs[!is_538], ...) +pkg_fix_remote_names <- function(pkgs) { + pkgs[pkgs == "fivethirtyeightdata"] <- "fivethirtyeightdata/fivethirtyeightdata" + pkgs } diff --git a/R/zzz.R b/R/zzz.R index d863356..63ddbce 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -1,5 +1,9 @@ .onAttach <- function(...) { - attached <- coursekata_attach(quietly = getOption("coursekata.quiet", FALSE) || quickstart()) + attached <- coursekata_attach( + do_not_ask = !interactive() || interactive() && quickstart(), + quietly = getOption("coursekata.quiet", FALSE) || quickstart() + ) + coursekata_load_theme() if (!quickstart()) { rlang::inform(coursekata_attach_message(attached), class = "packageStartupMessage") diff --git a/README.Rmd b/README.Rmd index 1f98df4..c6e42b3 100644 --- a/README.Rmd +++ b/README.Rmd @@ -34,19 +34,20 @@ This package makes it easy to install and load all packages and functions used i ## Installation +This package is available on CRAN and via the source on GitHub. In both cases, the installation is two-part: first install the package, then install the data packages `fivethirtyeight` and `fivethirtyeightdata`: + ```{r eval = FALSE} -# Install from CRAN install.packages("coursekata") +coursekata::coursekata_install() ``` ```{r eval = FALSE} # Install the development version from GitHub # install.packages("pak") pak::pak("coursekata/coursekata-r") +coursekata::coursekata_install() ``` -Note that installing the package will install all of the functions that are used in the course, but by default a couple of packages will not be installed: `fivethirtyeight` and `fivethirtyeightdata`. These packages only contain data, so the R package building process complains when functions are not imported from them. The first time you call `library(coursekata)` you will be prompted to install the packages if they are not already installed, or you can call `coursekata::coursekata_install()` to install them at any time. - ## Loading Packages Used in CourseKata Courses @@ -76,8 +77,9 @@ In addition to useful functions, a great deal of data sets are used by instructo ### Startup options -- By default, the package will show all startup messages from the dependent packages. To quiet these (like in the output above), you can set `options(coursekata.quiet = TRUE)` -- By default, a few checks are performed on startup to check that the packages are installed, up-to-date, and loading from the correct locations. This can take a couple of seconds; to disable this in environments where time is critical set `options(coursekata.quickstart = TRUE)` +- `coursekata.quickstart`: Each time the package is loaded (e.g. via `library(coursekata)`) a check is run to ensure that all the dependencies are installed and reasonably up-to-date. If they are not, you will be prompted to install missing packages. This can be disabled by setting `options(coursekata.quickstart = FALSE)`. + +- `coursekata.quiet`: By default, the package will show all startup messages from the dependent packages. To quiet these (like in the output above), you can set `options(coursekata.quiet = TRUE)` ## Functions and Theme diff --git a/README.md b/README.md index 28370f7..0e4985f 100644 --- a/README.md +++ b/README.md @@ -33,26 +33,22 @@ the [tidyverse](https://tidyverse.tidyverse.org) meta-package. ## Installation +This package is available on CRAN and via the source on GitHub. In both +cases, the installation is two-part: first install the package, then +install the data packages `fivethirtyeight` and `fivethirtyeightdata`: + ``` r -# Install from CRAN install.packages("coursekata") +coursekata::coursekata_install() ``` ``` r # Install the development version from GitHub # install.packages("pak") pak::pak("coursekata/coursekata-r") +coursekata::coursekata_install() ``` -Note that installing the package will install all of the functions that -are used in the course, but by default a couple of packages will not be -installed: `fivethirtyeight` and `fivethirtyeightdata`. These packages -only contain data, so the R package building process complains when -functions are not imported from them. The first time you call -`library(coursekata)` you will be prompted to install the packages if -they are not already installed, or you can call -`coursekata::coursekata_install()` to install them at any time. - ## Loading Packages Used in CourseKata Courses `library(coursekata)` will load the following core packages in addition @@ -93,14 +89,15 @@ instructors who teach the course. This package installs these: ### Startup options -- By default, the package will show all startup messages from the - dependent packages. To quiet these (like in the output above), you can - set `options(coursekata.quiet = TRUE)` -- By default, a few checks are performed on startup to check that the - packages are installed, up-to-date, and loading from the correct - locations. This can take a couple of seconds; to disable this in - environments where time is critical set - `options(coursekata.quickstart = TRUE)` +- `coursekata.quickstart`: Each time the package is loaded (e.g. via + `library(coursekata)`) a check is run to ensure that all the + dependencies are installed and reasonably up-to-date. If they are not, + you will be prompted to install missing packages. This can be disabled + by setting `options(coursekata.quickstart = FALSE)`. + +- `coursekata.quiet`: By default, the package will show all startup + messages from the dependent packages. To quiet these (like in the + output above), you can set `options(coursekata.quiet = TRUE)` ## Functions and Theme diff --git a/bin/load-non-interactive-quickstart.r b/bin/load-non-interactive-quickstart.r new file mode 100644 index 0000000..407a13d --- /dev/null +++ b/bin/load-non-interactive-quickstart.r @@ -0,0 +1,27 @@ +#!/usr/bin/env Rscript + +Sys.setenv(`_R_S3_METHOD_REGISTRATION_NOTE_OVERWRITES_` = "false") +options(coursekata.quiet = FALSE, coursekata.quickstart = TRUE, cli.width = 80) + +# ensure this package is uninstalled +suppressMessages(try(remove.packages("fivethirtyeightdata"), silent = TRUE)) + +# function to compare output to a snapshot +test_output_snapshot <- function(expr, snapshot) { + replace_smart_quotes <- function(x) { + x <- gsub("’", "'", x) + x <- gsub("‘", "'", x) + x <- gsub("“", '"', x) + x <- gsub("”", '"', x) + x + } + + output <- capture.output(expr, type = "message") + output <- trimws(output, "right") + output <- paste(output, collapse = "\n") + comp <- waldo::compare(replace_smart_quotes(output), replace_smart_quotes(snapshot)) + if (length(comp) > 0) stop(paste0(comp, collapse = "\n\n"), call. = FALSE) +} + +# loading should not trigger the install prompt or show any messages +test_output_snapshot(library(coursekata), "") diff --git a/bin/load-non-interactive-quiet.r b/bin/load-non-interactive-quiet.r new file mode 100644 index 0000000..c5f08b8 --- /dev/null +++ b/bin/load-non-interactive-quiet.r @@ -0,0 +1,33 @@ +#!/usr/bin/env Rscript + +Sys.setenv(`_R_S3_METHOD_REGISTRATION_NOTE_OVERWRITES_` = "false") +options(coursekata.quiet = TRUE, coursekata.quickstart = FALSE, cli.width = 80) + +# ensure this package is uninstalled +suppressMessages(try(remove.packages("fivethirtyeightdata"), silent = TRUE)) + +# function to compare output to a snapshot +test_output_snapshot <- function(expr, snapshot) { + replace_smart_quotes <- function(x) { + x <- gsub("’", "'", x) + x <- gsub("‘", "'", x) + x <- gsub("“", '"', x) + x <- gsub("”", '"', x) + x + } + + output <- capture.output(expr, type = "message") + output <- trimws(output, "right") + output <- paste(output, collapse = "\n") + comp <- waldo::compare(replace_smart_quotes(output), replace_smart_quotes(snapshot)) + if (length(comp) > 0) stop(paste0(comp, collapse = "\n\n"), call. = FALSE) +} + +# individual package start messages should not be printed (only the CourseKata message) +test_output_snapshot(library(coursekata), trimws(" +── CourseKata packages ──────────────────────────────────── coursekata 0.14.1 ── +✔ dslabs 0.7.6 ✔ Metrics 0.1.4 +✔ Lock5withR 1.2.2 ✔ lsr 0.5.2 +x fivethirtyeightdata ✔ mosaic 1.8.4.2 +✔ fivethirtyeight 0.6.2 ✔ supernova 2.5.7 +")) diff --git a/bin/load-non-interactive.r b/bin/load-non-interactive.r new file mode 100755 index 0000000..bada837 --- /dev/null +++ b/bin/load-non-interactive.r @@ -0,0 +1,71 @@ +#!/usr/bin/env Rscript + +Sys.setenv(`_R_S3_METHOD_REGISTRATION_NOTE_OVERWRITES_` = "false") +options(coursekata.quiet = FALSE, coursekata.quickstart = FALSE, cli.width = 80) + +# ensure this package is uninstalled +suppressMessages(try(remove.packages("fivethirtyeightdata"), silent = TRUE)) + +# function to compare output to a snapshot +test_output_snapshot <- function(expr, snapshot) { + replace_smart_quotes <- function(x) { + x <- gsub("’", "'", x) + x <- gsub("‘", "'", x) + x <- gsub("“", '"', x) + x <- gsub("”", '"', x) + x + } + + output <- capture.output(expr, type = "message") + output <- trimws(output, "right") + output <- paste(output, collapse = "\n") + comp <- waldo::compare(replace_smart_quotes(output), replace_smart_quotes(snapshot)) + if (length(comp) > 0) stop(paste0(comp, collapse = "\n\n"), call. = FALSE) +} + +test_output_snapshot(library(coursekata), trimws(" +Loading required package: dslabs +Loading required package: Lock5withR +Loading required package: fivethirtyeight +Some larger datasets need to be installed separately, like senators and +house_district_forecast. To install these, we recommend you install the +fivethirtyeightdata package by running: +install.packages('fivethirtyeightdata', repos = +'https://fivethirtyeightdata.github.io/drat/', type = 'source') +Loading required package: Metrics +Loading required package: lsr +Loading required package: mosaic + +The 'mosaic' package masks several functions from core packages in order to add +additional features. The original behavior of these functions should not be affected by this. + +Attaching package: ‘mosaic’ + +The following objects are masked from ‘package:dplyr’: + + count, do, tally + +The following object is masked from ‘package:Matrix’: + + mean + +The following object is masked from ‘package:ggplot2’: + + stat + +The following objects are masked from ‘package:stats’: + + binom.test, cor, cor.test, cov, fivenum, IQR, median, prop.test, + quantile, sd, t.test, var + +The following objects are masked from ‘package:base’: + + max, mean, min, prod, range, sample, sum + +Loading required package: supernova +── CourseKata packages ──────────────────────────────────── coursekata 0.14.1 ── +✔ dslabs 0.7.6 ✔ Metrics 0.1.4 +✔ Lock5withR 1.2.2 ✔ lsr 0.5.2 +x fivethirtyeightdata ✔ mosaic 1.8.4.2 +✔ fivethirtyeight 0.6.2 ✔ supernova 2.5.7 +")) diff --git a/cran-comments.md b/cran-comments.md index 4a73dad..433216b 100644 --- a/cran-comments.md +++ b/cran-comments.md @@ -1,27 +1,23 @@ ## Release summary -- This is a new package. This is the second time submitting this package to CRAN, and it has been - updated to address the feedback from the first submission. +Package was failing on CRAN checks and asking for user input in non-interactive mode. The check is for a missing package that should be installed for full functionality, but is not required. The fix was to add a check for interactive mode before asking for user input. -- Requested changes: +Additional comments from Uwe Ligges were that the user prompt was confusing. Instead of using `yesno` to generate the prompt, we are now using `rlang::check_installed()` to handle the prompt and subsequent installation. - - The Title and Description now use single quotes around 'CourseKata' - - All exported functions have been checked to ensure that their return values - are documented in the `@return` section of the function documentation. - - coursekata_load_theme - - coursekata_unload_theme - - gf_model +Other changes: -- Additional changes: - - Removed internal docs and opted for @noRd tags as suggested by tidyverse +- Reduce calls to `pak::pkg_status()` to improve startup time +- Address issue where `require(lib.loc = ...)` was sometimes being passed `NA` instead of `NULL` ## Test environments - Local install on macOS Sonoma 14.0 (ARM); R 4.3.1 - GitHub Actions - - macOS: 12.7; R: 4.3.1 + - macOS: 12.6.9; R: 4.3.1 - Microsoft Windows Server 2022: 10.0.20348; R: 4.3.1, 3.6.3 - Ubuntu: 22.04.3; R: devel, 4.3.1, 4.2.3 +- devtools::check_rhub() +- devtools::check_win_devel() ## R CMD check results diff --git a/man/coursekata_attach.Rd b/man/coursekata_attach.Rd index d1d42d5..294e418 100644 --- a/man/coursekata_attach.Rd +++ b/man/coursekata_attach.Rd @@ -4,9 +4,11 @@ \alias{coursekata_attach} \title{Attach the CourseKata course packages} \usage{ -coursekata_attach(quietly = FALSE) +coursekata_attach(do_not_ask = FALSE, quietly = FALSE) } \arguments{ +\item{do_not_ask}{Prevent asking the user to install missing packages (they are skipped).} + \item{quietly}{Whether to suppress messages.} } \value{ diff --git a/man/figures/README-samp_dist_of_b1-1.png b/man/figures/README-samp_dist_of_b1-1.png index 00fc1be..8c33a20 100644 Binary files a/man/figures/README-samp_dist_of_b1-1.png and b/man/figures/README-samp_dist_of_b1-1.png differ diff --git a/man/figures/README-samp_dist_of_hp-1.png b/man/figures/README-samp_dist_of_hp-1.png index 72f60a0..c2bbb02 100644 Binary files a/man/figures/README-samp_dist_of_hp-1.png and b/man/figures/README-samp_dist_of_hp-1.png differ diff --git a/tests/testthat/test-coursekata_attach.R b/tests/testthat/test-coursekata_attach.R index 2c63802..8777506 100644 --- a/tests/testthat/test-coursekata_attach.R +++ b/tests/testthat/test-coursekata_attach.R @@ -1,16 +1,19 @@ test_that("all course packages are listed with version and whether attached", { - # use these packages because they are not imported - # they need to be in the order they appear in the coursekata_pkgs object + # these packages are not imported, but are still in the load out + # (important because they can be detached and re-attached during testing) pkgs <- c("fivethirtyeightdata", "fivethirtyeight") - # fivethirtyeightdata is not always installed, and if so, it won't get attached - # detach/re-attach only if it is installed + + # they need to be in the order they appear in the coursekata_pkgs object + pkgs <- pkgs[order(match(pkgs, coursekata_pkgs))] + + # detach/re-attach only installed packages installed <- pkg_is_installed(pkgs) + names(installed) <- pkgs detacher(pkgs[installed]) withr::defer(attacher(pkgs[installed])) # only the installed package will be attached attachments <- coursekata_attach(quietly = TRUE) - names(installed) <- pkgs expect_identical(attachments, installed) }) diff --git a/tests/testthat/test-utils-pkg.R b/tests/testthat/test-utils-pkg.R index 28cfb67..ac57634 100644 --- a/tests/testthat/test-utils-pkg.R +++ b/tests/testthat/test-utils-pkg.R @@ -1,9 +1,16 @@ test_that("it determines whether a package is attached or not", { - pkgs <- "fivethirtyeight" # use this package because it is not imported + # use a package that can be unloaded and reloaded (so, can't be imported by coursekata) + pkgs <- "fivethirtyeight" + + # make sure the package can be added + if (!require(pkgs, character.only = TRUE, quietly = TRUE)) { + fail(paste("Package not available:", pkgs)) + } purrr::walk(pkgs, detacher) - expect_vector(pkg_is_attached(pkgs), logical(), length(pkgs)) - expect_true(all(!pkg_is_attached(pkgs))) + attached <- pkg_is_attached(pkgs) + expect_vector(attached, logical(), length(pkgs)) + expect_true(all(!attached)) # reattach for other tests purrr::walk(pkgs, attacher) @@ -11,14 +18,6 @@ test_that("it determines whether a package is attached or not", { }) -test_that("it retrieves the library location for currently installed packages, or NA", { - pkgs <- c("supernova", "lsr", "does_not_exist") - locations <- pkg_library_location(pkgs) - expect_identical(unname(locations[3]), NA_character_) - expect_true(all(dir.exists(locations[1:2]))) -}) - - test_that("it retrieves the package version for currently installed packages, or NA", { pkgs <- c("supernova", "lsr", "does-not-exist") expect_vector(pkg_version(pkgs), character(), length(pkgs)) @@ -39,20 +38,6 @@ test_that("requiring a package can be quiet", { expect_message(pkg_require(pkgs, quietly = TRUE), NA) }) -test_that("requiring a missing package calmly asks if you want to install it", { - skip("Make sure to run this when testing locally.") - - menu_mock <- mockery::mock(FALSE) - mockr::with_mock( - .env = as.environment("package:coursekata"), - ask_to_install = menu_mock, - { - expect_warning(pkg_require("does not exist"), NA) - expect_length(mockery::mock_calls(menu_mock), 1) - } - ) -}) - test_that("installing fivethirtyeightdata works as intended", { skip("Make sure to run this when testing locally.") @@ -64,6 +49,6 @@ test_that("installing fivethirtyeightdata works as intended", { error = function(e) invisible(NULL) ) - pkg_install("fivethirtyeightdata") + pkg_check_installed("fivethirtyeightdata") expect_true(require("fivethirtyeightdata")) })