Release touchstone 0.1.0

.Rbuildignore

@@ -10,3 +10,5 @@
 ^pkgdown$
 ^doc$
 ^Meta$
+^cran-comments\.md$
+^touchstone$

DESCRIPTION

@@ -1,7 +1,7 @@
 Type: Package
 Package: touchstone
 Title: Continuous Benchmarking with statistical confidence based on 'Git' branches
-Version: 0.0.0.9001
+Version: 0.1.3
 Authors@R: 
     c(person(given = "Lorenz",
              family = "Walthert",
@@ -14,12 +14,14 @@ Authors@R:
              comment = c(ORCID = "0000-0002-7281-3989")))
 Description: A common problem of benchmarking in continuous integration is
     that the computational power of the virtual machines that run the job
-    varies over time.  This package allows users to benchmark two git refs
-    of the same repo in a random sequence for better comparison.
+    varies over time. This package runs benchmarking code of two 'Git'
+    refs of the same repository in a random sequence for better
+    comparison, either locally or via 'GitHub Actions' and outputs a
+    confidence interval for the relative difference.
 License: MIT + file LICENSE
-URL: https://github.com/lorenzwalthert/touchstone,
-    https://lorenzwalthert.github.io/touchstone
-BugReports: https://github.com/lorenzwalthert/touchstone/issues
+URL: https://github.com/lorenzwalthert/touchstone/,
+    https://lorenzwalthert.github.io/touchstone/
+BugReports: https://github.com/lorenzwalthert/touchstone/issues/
 Imports: 
     bench,
     callr,
@@ -48,7 +50,6 @@ VignetteBuilder:
 Config/testthat/edition: 3
 Config/testthat/parallel: true
 Encoding: UTF-8
-LazyData: true
 Roxygen: list(markdown = TRUE, roclets = c("rd", "namespace", "collate",
     if (rlang::is_installed("pkgapi")) "pkgapi::api_roclet" else {
     warning("Please install r-lib/pkgapi to make sure the file API is kept

NEWS.md

@@ -1,4 +1,4 @@
-# touchstone 0.1.0-rc
+# touchstone 0.1.3
 
 This is the initial CRAN release of {touchstone}. Please see the README.md on 
 how to get started.

R/analyze.R

@@ -96,6 +96,8 @@ benchmark_analyze <- function(benchmark, refs = c(
 #'
 #' `refs` must be passed because the order is relevant.
 #' @inheritParams benchmark_plot
+#' @return
+#' A character vector with the text that verbalizes the benchmark.
 #' @keywords internal
 benchmark_verbalize <- function(benchmark, timings, refs, ci) {
   tbl <- timings %>%
@@ -160,6 +162,8 @@ confint_relative_get <- function(timings, refs, reference, ci) {
 
 #' @param timing a benchmark read with [benchmark_read()], column `name` must
 #'   only contain one unique value.
+#' @return
+#' The function is called for its side effects and returns `NULL` (invisibly).
 #' @keywords internal
 benchmark_plot <- function(benchmark, timings) {
   timings %>%

R/core.R

@@ -5,6 +5,8 @@
 #' @param n Number of iterations to run a benchmark within an iteration.
 #' @param dots list of quoted expressions (length 1).
 #' @inheritParams benchmark_write
+#' @return
+#' A tibble with the benchmarks.
 #' @importFrom tibble lst tibble
 #' @keywords internal
 benchmark_run_iteration <- function(expr_before_benchmark,
@@ -109,6 +111,8 @@ benchmark_run_ref <- function(expr_before_benchmark =
 #'
 #' @param path_pkg The path to the root of the package you want to benchmark.
 #' @inheritParams benchmark_run_iteration
+#' @return
+#' A tibble with the benchmarks.
 #' @keywords internal
 benchmark_run_ref_impl <- function(ref,
                                    block,

R/testing.R

@@ -4,6 +4,8 @@
 #' @inheritParams withr::defer
 #' @family testers
 #' @keywords internal
+#' @return
+#' The output of `withr::defer()`, i.e. a list with `expr` and `envir`.
 local_clean_touchstone <- function(envir = parent.frame()) {
   withr::defer(touchstone_clear(all = TRUE), envir = envir)
 }
@@ -27,6 +29,8 @@ path_temp_pkg <- function(name) {
 #'   to validate if the installed package corresponds to source branch for
 #'   testing. If `NULL`, nothing is written.
 #' @inheritParams withr::defer
+#' @return
+#' Character vector of length one with path to package.
 #' @family testers
 #' @keywords internal
 local_package <- function(pkg_name = fs::path_file(fs::file_temp("pkg")),
@@ -54,7 +58,7 @@ local_package <- function(pkg_name = fs::path_file(fs::file_temp("pkg")),
   gert::git_add("R/")
   gert::git_commit("[init]")
   branches <- gert::git_branch_list() %>%
-    dplyr::pull(name) %>%
+    dplyr::pull(.data$name) %>%
     dplyr::setdiff(branches, .)
   purrr::walk(branches, gert::git_branch_create)
   withr::defer(unlink(path), envir = envir)

R/utils.R

@@ -7,7 +7,9 @@ NULL
 #' @describeIn touchstone_managers returns the directory where the touchstone database lives.
 #' @aliases touchstone_managers
 #' @return
-#' Character vector of length one with th path to the touchstone directory.
+#' Character vector of length one with the path to the touchstone directory (for
+#' `dir_touchstone()`), path to the deleted files (for `touchstone_clear()`) or
+#' path to the file containing the pull request comment (for `path_pr_comment()`).
 #' @export
 dir_touchstone <- function() {
   getOption("touchstone.dir", "touchstone")
@@ -49,8 +51,6 @@ path_touchstone_script <- function() {
 #' @aliases touchstone_managers
 #' @param all Whether to clear the whole touchstone directory or just the
 #'   records sub directory.
-#' @return
-#' The deleted paths (invisibly).
 #' @export
 touchstone_clear <- function(all = FALSE) {
   paths <- fs::path(dir_touchstone(), if (!all) c("records", "lib") else "")
@@ -67,6 +67,8 @@ touchstone_clear <- function(all = FALSE) {
 #' always perform worse than the second, so the order within the blocks must be
 #' random.
 #' @keywords internal
+#' @return
+#' A tibble with columns `block` and `ref`.
 ref_upsample <- function(ref, n = 20) {
   purrr::map_dfr(
     rlang::seq2(1, n),
@@ -121,6 +123,8 @@ local_git_checkout <- function(branch,
 #'
 #' Advantages: Keep benchmarked repo in touchstone library only.
 #' @keywords internal
+#' @return
+#' The old library paths (invisibly).
 local_without_touchstone_lib <- function(path_pkg = ".", envir = parent.frame()) {
   all <- normalizePath(.libPaths())
   is_touchstone <- fs::path_has_parent(
@@ -133,6 +137,9 @@ local_without_touchstone_lib <- function(path_pkg = ".", envir = parent.frame())
 
 #' Make sure there is no installation of the package to benchmark in the global
 #' package library
+#' @param path_pkg The path to the package to check.
+#' @return
+#' `TRUE` invisibly or an error if there is a global installation.
 #' @keywords internal
 assert_no_global_installation <- function(path_pkg = ".") {
   local_without_touchstone_lib()
@@ -151,13 +158,31 @@ assert_no_global_installation <- function(path_pkg = ".") {
 
 
 #' Check if a package is installed and unloading it
+#' @return
+#' A list with `name` of the package to check and `installed`, a boolean to
+#' indicate if the package is installed or not.
 #' @keywords internal
 is_installed <- function(path_pkg = ".") {
   path_desc <- fs::path(path_pkg, "DESCRIPTION")
   pkg_name <- unname(read.dcf(path_desc)[, "Package"])
   list(
     name = pkg_name,
-    installed = pkg_name %in% rownames(utils::installed.packages())
+    installed = is_installed_impl(pkg_name)
+  )
+}
+
+# When a package is removed from the library with [remove.packages()] and then
+# loaded e.g. with [devtools::load_all()],
+# [rlang::is_installed()] and similar returns `TRUE`,
+# `pkg_name %in% installed.packages()` `FALSE` (but CRAN does not want to see
+# `installed.packages()` used since it may be slow).
+is_installed_impl <- function(pkg_name) {
+  rlang::with_handlers(
+    {
+      find.package(pkg_name, lib.loc = .libPaths())
+      TRUE
+    },
+    error = ~FALSE
   )
 }
 
@@ -311,10 +336,8 @@ get_asset_dir <- function(ref) {
 }
 
 
-#' @describeIn touchstone_managers returns the path to the file containing the pr comment.
+#' @describeIn touchstone_managers Returns the path to the file containing the pull request comment.
 #' @aliases touchstone_managers
-#' @return
-#' Character vector of length one with the path to the pr comment.
 #' @export
 #' @seealso [pr_comment]
 path_pr_comment <- function() {

README.Rmd

@@ -3,7 +3,7 @@ output: github_document
 ---
 
 <!-- badges: start --> [![Lifecycle:
-experimental](https://img.shields.io/badge/lifecycle-experimental-orange.svg)](https://www.tidyverse.org/lifecycle/#experimental)
+experimental](https://img.shields.io/badge/lifecycle-experimental-orange.svg)](https://lifecycle.r-lib.org/articles/stages.html#experimental)
 [![R build
 status](https://github.com/lorenzwalthert/touchstone/workflows/R-CMD-check/badge.svg)](https://github.com/lorenzwalthert/touchstone/actions)
 <!-- badges: end -->
@@ -67,7 +67,7 @@ package developers. The following insights were the motivation:
   least when benchmark results don't vary significantly (> 50%).
 
 - **Dependencies should be identical across versions:** R and package versions
-  of dependencies must be fixed via [RSPM](http://packagemanager.rstudio.com) to
+  of dependencies must be fixed via [RSPM](https://packagemanager.rstudio.com/client/#/) to
   allow as much continuation as possible anyways. Changing the timestamp of RSPM
   can happen in PRs that are only dedicated to dependency updates.
 

README.md

@@ -2,7 +2,7 @@
 <!-- badges: start -->
 
 [![Lifecycle:
-experimental](https://img.shields.io/badge/lifecycle-experimental-orange.svg)](https://www.tidyverse.org/lifecycle/#experimental)
+experimental](https://img.shields.io/badge/lifecycle-experimental-orange.svg)](https://lifecycle.r-lib.org/articles/stages.html#experimental)
 [![R build
 status](https://github.com/lorenzwalthert/touchstone/workflows/R-CMD-check/badge.svg)](https://github.com/lorenzwalthert/touchstone/actions)
 <!-- badges: end -->
@@ -68,9 +68,10 @@ motivation:
 
 -   **Dependencies should be identical across versions:** R and package
     versions of dependencies must be fixed via
-    [RSPM](http://packagemanager.rstudio.com) to allow as much
-    continuation as possible anyways. Changing the timestamp of RSPM can
-    happen in PRs that are only dedicated to dependency updates.
+    [RSPM](https://packagemanager.rstudio.com/client/#/) to allow as
+    much continuation as possible anyways. Changing the timestamp of
+    RSPM can happen in PRs that are only dedicated to dependency
+    updates.
 
 -   **Pull requests are a natural unit for measuring speed:** Linking
     benchmarking to pull requests make sense because you can easily

cran-comments.md

@@ -0,0 +1,47 @@
+This is a re-submission based on the feedback from Gregor Seyer received on June
+18, 2021.
+
+## Test environments
+
+* local R installation, R 4.1.0
+* ubuntu 18.04 (on GitHub Actions), R 4.1.0 as well as R devel.
+* Windows Server 10 (on GitHub Actions): R 4.1.0.
+* win-builder (devel)
+
+## R CMD check results
+
+0 errors | 0 warnings | 2 note
+
+* This is a new release.
+* Found the following calls to attach():
+    File 'touchstone/R/core.R':
+    attach(loadNamespace("touchstone"), name = new_name)
+     See section 'Good practice' in '?attach'.
+
+For the latter, I followed the good practices. My package needs to be invoked
+from a separate R process (via the R package callr) because library paths 
+cannot be cleanly removed within an R session and this function requires that
+temporarily. Alternatively, I could have exported `exprs_eval` and called it
+from that sub-process with `::`, but I prefer not to export it since it's not a
+user-facing function.
+
+
+In addition, I addressed all comments by Gregor Seyer. 
+
+
+* add \value to .Rd files regarding exported methods. 
+
+> I already had that for all exported methods. You are referring to unexported
+methods with the keyword internal. I think the check should be adapted to only 
+require return value documentation for exported methods (or adapt the standard
+text above to include all documented objects).
+
+* You have examples for unexported functions. `is_installed()`.
+
+> This is a false positive. I was using `rlang::is_installed()` in an example, 
+not the function `is_installed()` from my own package. Nevertheless, I worked 
+around this.
+
+* You are using installed.packages():
+
+> Thanks, I use your suggested way of checking now if a package is installed.

inst/WORDLIST

@@ -39,6 +39,7 @@ getwd
 ggplot
 github
 gitignore
+Gregor
 hashFiles
 href
 https
@@ -57,6 +58,7 @@ LIBS
 Lifecycle
 linux
 listWorkflowRunArtifacts
+loadNamespace
 lorenz
 lorenzwalthert
 magrittr
@@ -100,6 +102,7 @@ runif
 saveRDS
 seealso
 sep
+Seyer
 SHA
 sideeffects
 sprintf