Testing workflow changes and commit SHAs

.ci/atime/tests.R

@@ -17,66 +17,41 @@
 # @note Please check https://github.com/tdhock/atime/blob/main/vignettes/data.table.Rmd for more information.
 # nolint start: undesirable_operator_linter. ':::' needed+appropriate here.
 test.list <- atime::atime_test_list(
-  # A function to customize R package metadata and source files to facilitate version-specific installation and testing.
-  #
-  # This is specifically tailored for handling data.table which requires specific changes in non-standard files (such as the object file name in Makevars and version checking code in onLoad.R)
-  # to support testing across different versions (base and HEAD for PRs, commits associated with historical regressions, etc.) of the package.
-  # It appends a SHA1 hash to the package name (PKG.SHA), ensuring each version can be installed and tested separately.
-  #
-  # @param old.Package Current name of the package.
-  # @param new.Package New name of the package, including a SHA hash.
-  # @param sha SHA1 hash used for differentiating versions.
-  # @param new.pkg.path Path to the package files.
-  #
-  # @details
-  # The function modifies:
-  # - DESCRIPTION, updating the package name.
-  # - Makevars, customizing the shared object file name and adjusting the build settings.
-  # - R/onLoad.R, adapting custom version checking for package loading operations.
-  # - NAMESPACE, changing namespace settings for dynamic linking.
-  #
-  # @examples
-  # pkg.edit.fun("data.table", "data.table.some_SHA1_hash", "some_SHA1_hash", "/path/to/data.table")
-  #
-  # @return None (performs in-place file modifications)
-  # @note This setup is typically unnecessary for most packages but essential for data.table due to its unique configuration.
   pkg.edit.fun = function(old.Package, new.Package, sha, new.pkg.path) {
-    pkg_find_replace <- function(glob, FIND, REPLACE) {
-      atime::glob_find_replace(file.path(new.pkg.path, glob), FIND, REPLACE)
-    }
-    Package_regex <- gsub(".", "_?", old.Package, fixed = TRUE)
-    Package_ <- gsub(".", "_", old.Package, fixed = TRUE)
-    new.Package_ <- paste0(Package_, "_", sha)
-    pkg_find_replace(
-      "DESCRIPTION",
-      paste0("Package:\\s+", old.Package),
-      paste("Package:", new.Package))
-    pkg_find_replace(
-      file.path("src", "Makevars.*in"),
-      Package_regex,
-      new.Package_)
-    pkg_find_replace(
-      file.path("R", "onLoad.R"),
-      Package_regex,
-      new.Package_)
-    pkg_find_replace(
-      file.path("R", "onLoad.R"),
-      sprintf('packageVersion\\("%s"\\)', old.Package),
-      sprintf('packageVersion\\("%s"\\)', new.Package))
-    pkg_find_replace(
-      file.path("src", "init.c"),
-      paste0("R_init_", Package_regex),
-      paste0("R_init_", gsub("[.]", "_", new.Package_)))
-    pkg_find_replace(
-      "NAMESPACE",
-      sprintf('useDynLib\\("?%s"?', Package_regex),
-      paste0('useDynLib(', new.Package_))
-  },
-
-  # Performance regression discussed in: https://github.com/Rdatatable/data.table/issues/4311
-  # Fixed in: https://github.com/Rdatatable/data.table/pull/4440
+      pkg_find_replace <- function(glob, FIND, REPLACE) {
+        atime::glob_find_replace(file.path(new.pkg.path, glob), FIND, REPLACE)
+      }
+      Package_regex <- gsub(".", "_?", old.Package, fixed = TRUE)
+      Package_ <- gsub(".", "_", old.Package, fixed = TRUE)
+      new.Package_ <- paste0(Package_, "_", sha)
+      pkg_find_replace(
+        "DESCRIPTION",
+        paste0("Package:\\s+", old.Package),
+        paste("Package:", new.Package))
+      pkg_find_replace(
+        file.path("src", "Makevars.*in"),
+        Package_regex,
+        new.Package_)
+      pkg_find_replace(
+        file.path("R", "onLoad.R"),
+        Package_regex,
+        new.Package_)
+      pkg_find_replace(
+        file.path("R", "onLoad.R"),
+        sprintf('packageVersion\\("%s"\\)', old.Package),
+        sprintf('packageVersion\\("%s"\\)', new.Package))
+      pkg_find_replace(
+        file.path("src", "init.c"),
+        paste0("R_init_", Package_regex),
+        paste0("R_init_", gsub("[.]", "_", new.Package_)))
+      pkg_find_replace(
+        "NAMESPACE",
+        sprintf('useDynLib\\("?%s"?', Package_regex),
+        paste0('useDynLib(', new.Package_))
+    },    
+
   "shallow regression fixed in #4440" = atime::atime_test(
-    N = 10^seq(3,8),
+    N = 10^seq(3, 8),
     setup = {
       set.seed(1L)
       dt <- data.table(a = sample.int(N))
@@ -119,6 +94,16 @@ test.list <- atime::atime_test_list(
       data.table:::setDT(L)
     },
     Slow = "c4a2085e35689a108d67dacb2f8261e4964d7e12", # Parent of the first commit in the PR that fixes the issue (https://github.com/Rdatatable/data.table/commit/7cc4da4c1c8e568f655ab5167922dcdb75953801)
-    Fast = "1872f473b20fdcddc5c1b35d79fe9229cd9a1d15") # Last commit in the PR that fixes the issue (https://github.com/Rdatatable/data.table/pull/5427/commits)
+    Fast = "1872f473b20fdcddc5c1b35d79fe9229cd9a1d15"), # Last commit in the PR that fixes the issue (https://github.com/Rdatatable/data.table/pull/5427/commits) for my fork here.
+    # Merge commit in the PR that fixes the issue (https://github.com/Rdatatable/data.table/pull/5427) # <-- This too would probably work but in my fork I don't have any commits beyond 1872f473b20fdcddc5c1b35d79fe9229cd9a1d15 for that branch,
+
+  # Issue reported in: https://github.com/Rdatatable/data.table/issues/6286
+  "by with verbose = TRUE" = atime::atime_test(
+    N = 10^seq(1, 8),
+    setup = {
+    dt = data.table(a = 1:N)
+  },
+ expr = data.table:::copy(dt)[, 1, by = a, verbose = TRUE],
+  "Slow" = "a01f00f7438daf4612280d6886e6929fa8c8f76e",
+  "Fast" = "cd497408bb4dc6650d871b5076e738420ff431d7")
 )
-# nolint end: undesirable_operator_linter.

.github/workflows/autocomment.yml

@@ -0,0 +1,21 @@
+name: Autocomment atime-based performance regression analysis on PRs
+
+on:
+  pull_request:
+    branches:
+      - '*'
+    types:
+      - opened
+      - reopened
+      - synchronize
+
+jobs:
+  comment:
+    runs-on: ubuntu-latest
+    container: ghcr.io/iterative/cml:0-dvc2-base1
+    env:
+      GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
+      repo_token: ${{ secrets.GITHUB_TOKEN }}
+      R_KEEP_PKG_SOURCE: yes
+    steps:
+      - uses: Anirban166/Autocomment-atime-results@v1.1.0

src/negate.c

@@ -11,7 +11,6 @@ void negateByRef(SEXP x) {
   }
 }
 
-
 SEXP notchin(SEXP x, SEXP table) {
   // see discussion in PR#4931
   SEXP result = PROTECT(chin(x, table));