Merge branch 'master' into litedown

Author: ben-schwen

.ci/README.md

@@ -12,11 +12,15 @@ Test jobs:
 - `test-lin-rel-cran` - `--as-cran` on Linux, strict test for final status of `R CMD check`.
 - `test-lin-dev-gcc-strict-cran` - `--as-cran` on Linux, `r-devel` built with `-enable-strict-barrier --disable-long-double`, test for compilation warnings, test for new NOTEs/WARNINGs from `R CMD check`.
 - `test-lin-dev-clang-cran` - same as `gcc-strict` job but R built with `clang` and  no `--enable-strict-barrier --disable-long-double` flags.
-- `test-lin-310-cran` - R 3.1.0 on Linux, stated R dependency version.
+- `test-lin-ancient-cran` - Stated R dependency version (currently 3.3.0) on Linux.
+- `test-lin-dev-san` - `r-devel` on Linux built with `clang -fsanitize=address,undefined` (including LeakSanitizer), test for sanitizer output in tests and examples.
 - `test-win-rel` - `r-release` on Windows.
 - `test-win-dev` - `r-devel` on Windows.
 - `test-win-old` - `r-oldrel` on Windows.
-- `test-mac-rel` - macOS build not yet available, see [#3326](https://github.com/Rdatatable/data.table/issues/3326) for status
+- `test-mac-rel` - `r-release` on macOS.
+- `test-mac-old` - `r-oldrel` on macOS.
+
+The CI steps for the tests are [required](https://github.com/Rdatatable/data.table/blob/55eb0f160b169398d51f138131c14a66c86e5dc9/.ci/publish.R#L162-L168) to be named according to the pattern `test-(lin|win|mac)-<R version>[-<suffix>]*`, where `<R version>` is `rel`, `dev`, `old`, `ancient`, or three digits comprising an R version (e.g. `362` corresponding to R-3.6.2).
 
 Tests jobs are allowed to fail, summary and logs of test jobs are later published at _CRAN-like checks_ page, see artifacts below.
 

.dev/lsan.supp

@@ -0,0 +1 @@
+leak:libfontconfig.so

.gitlab-ci.yml

@@ -212,6 +212,24 @@ test-lin-ancient-cran:
     # Restore checking vignettes if upgrading our R dependency means knitr can be installed.
     - R CMD check --no-manual --no-build-vignettes --ignore-vignettes $(ls -1t data.table_*.tar.gz | head -n 1)
 
+# run the main checks with Address(+Leak),UBSanitizer enabled
+test-lin-dev-san:
+  <<: *test-lin
+  image: registry.gitlab.com/rdatatable/dockerfiles/r-devel-clang-san
+  variables:
+    # must be set for most of the process because there are pseudo-leaks everywhere
+    ASAN_OPTIONS: "detect_leaks=0"
+    # fontconfig is known to leak; add more suppressions as discovered
+    LSAN_OPTIONS: "suppressions=$CI_PROJECT_DIR/.dev/lsan.supp"
+    UBSAN_OPTIONS: "print_stacktrace=1"
+  script:
+    - *install-deps
+    - |
+        res1=0; ASAN_OPTIONS=detect_leaks=1 R CMD check --no-manual $(ls -1t data.table_*.tar.gz | head -n 1) || res1=$?
+        res2=0; perl -nle '(print, $a=1) if /: runtime error: |ERROR: LeakSanitizer/../SUMMARY.*Sanitizer/ }{ exit $a' data.table.Rcheck/**/*.Rout* || res2=$?
+        # fail if R CMD check had failed or if sanitizer output found
+        if [ $res1 -ne 0 ] || [ $res2 -ne 0 ]; then exit 1; fi
+
 .test-win-template: &test-win
   <<: *test
   tags:
@@ -311,7 +329,7 @@ integration:
     - saas-linux-medium-amd64
   only:
     - master
-  needs: ["mirror-packages","build","test-lin-rel","test-lin-rel-cran","test-lin-dev-gcc-strict-cran","test-lin-dev-clang-cran","test-lin-rel-vanilla","test-lin-ancient-cran","test-win-rel","test-win-dev" ,"test-win-old","test-mac-rel","test-mac-old"]
+  needs: ["mirror-packages","build","test-lin-rel","test-lin-rel-cran","test-lin-dev-gcc-strict-cran","test-lin-dev-clang-cran","test-lin-rel-vanilla","test-lin-ancient-cran","test-lin-dev-san","test-win-rel","test-win-dev" ,"test-win-old","test-mac-rel","test-mac-old"]
   script:
     - R --version
     - *install-deps ## markdown pkg not present in r-pkgdown image

NEWS.md

@@ -12,10 +12,23 @@
 
 2. `fwrite(compress="gzip")` once again produces a gzip header when the column names are missing or disabled, [@6852](https://github.com/Rdatatable/data.table/issues/6852). Thanks @maxscheiber for the report and @aitap for the fix.
 
+3. `fread(keepLeadingZeros=TRUE)` now correctly parses dates with components with leading zeros as dates instead of strings, [#6851](https://github.com/Rdatatable/data.table/issues/6851). Thanks @TurnaevEvgeny for the report and @ben-schwen for the fix.
+
+4. `as.data.table()` now properly handles keys: specifying keys sets them, omitting keys preserves existing ones, and setting `key=NULL` clears them, [#6859](https://github.com/Rdatatable/data.table/issues/6859). Thanks @brookslogan for the report and @Mukulyadav2004 for the fix.
+
+5. `as.data.table()` on `x` avoids an infinite loop if the output of the corresponding `as.data.frame()` method has the same class as the input, [#6874](https://github.com/Rdatatable/data.table/issues/6874). Concretely, we had `class(x) = c('foo', 'data.frame')` and `class(as.data.frame(x)) = c('foo', 'data.frame')`, so `as.data.frame.foo` wound up getting called repeatedly. Thanks @matschmitz for the report and @ben-schwen for the fix.
+
 ## NOTES
 
 1. Continued work to remove non-API C functions, [#6180](https://github.com/Rdatatable/data.table/issues/6180). Thanks Ivan Krylov for the PRs and for writing a clear and concise guide about the R API: https://aitap.codeberg.page/R-api/.
 
+2. The following in-progress deprecations have proceeded:
+
+   + Argument `logicalAsInt` to `fwrite()` has been removed.
+   + Argument `autostart` to `fread()` has been removed.
+   + Argument `in.place` to `droplevels` has been removed.
+   + It's now an error to set `datatable.nomatch`, which has been warning since 1.15.0.
+
 # data.table [v1.17.0](https://github.com/Rdatatable/data.table/milestone/34)  (20 Feb 2025)
 
 ## POTENTIALLY BREAKING CHANGES

R/as.data.table.R

@@ -214,8 +214,13 @@ as.data.table.list = function(x,
 }
 
 as.data.table.data.frame = function(x, keep.rownames=FALSE, key=NULL, ...) {
-  if (is.data.table(x)) return(as.data.table.data.table(x)) # S3 is weird, #6739. Also # nocov; this is tested in 2302.{2,3}, not sure why it doesn't show up in coverage.
-  if (!identical(class(x), "data.frame")) return(as.data.table(as.data.frame(x)))
+  if (is.data.table(x)) return(as.data.table.data.table(x, key=key)) # S3 is weird, #6739. Also # nocov; this is tested in 2302.{2,3}, not sure why it doesn't show up in coverage.
+  if (!identical(class(x), "data.frame")) {
+    class_orig = class(x)
+    x = as.data.frame(x)
+    if (identical(class(x), class_orig)) setattr(x, "class", "data.frame") # cater for cases when as.data.frame can generate a loop #6874
+    return(as.data.table.data.frame(x, keep.rownames=keep.rownames, key=key, ...))
+  }
   if (!isFALSE(keep.rownames)) {
     # can specify col name to keep.rownames, #575; if it's the same as key,
     #   kludge it to 'rn' since we only apply the new name afterwards, #4468
@@ -228,7 +233,7 @@ as.data.table.data.frame = function(x, keep.rownames=FALSE, key=NULL, ...) {
   if (any(cols_with_dims(x))) {
     # a data.frame with a column that is data.frame needs to be expanded; test 2013.4
     # x may be a class with [[ method that behaves differently, so as.list first for default [[, #4526
-    return(as.data.table.list(as.list(x), keep.rownames=keep.rownames, ...))
+    return(as.data.table.list(as.list(x), keep.rownames=keep.rownames, key = key,...))
   }
   ans = copy(x)  # TO DO: change this deep copy to be shallow.
   setattr(ans, "row.names", .set_row_names(nrow(x)))
@@ -245,13 +250,14 @@ as.data.table.data.frame = function(x, keep.rownames=FALSE, key=NULL, ...) {
   ans
 }
 
-as.data.table.data.table = function(x, ...) {
+as.data.table.data.table = function(x, ..., key=NULL) {
   # as.data.table always returns a copy, automatically takes care of #473
   if (any(cols_with_dims(x))) { # for test 2089.2
-    return(as.data.table.list(x, ...))
+    return(as.data.table.list(x, key = key, ...))
   }
   x = copy(x) # #1681
   # fix for #1078 and #1128, see .resetclass() for explanation.
   setattr(x, 'class', .resetclass(x, "data.table"))
+  if (!missing(key)) setkeyv(x, key)
   x
 }

R/data.table.R

@@ -448,7 +448,7 @@ replace_dot_alias = function(e) {
     if (is.data.frame(i)) {
       if (missing(on)) {
         if (!haskey(x)) {
-          stopf("When i is a data.table (or character vector), the columns to join by must be specified using 'on=' argument (see ?data.table), by keying x (i.e. sorted, and, marked as sorted, see ?setkey), or by sharing column names between x and i (i.e., a natural join). Keyed joins might have further speed benefits on very large data due to x being sorted in RAM.")
+          stopf("When i is a data.table (or character vector), the columns to join by must be specified using the 'on=' argument (see ?data.table); by keying x (i.e., x is sorted and marked as such, see ?setkey); or by using 'on = .NATURAL' to indicate using the shared column names between x and i (i.e., a natural join). Keyed joins might have further speed benefits on very large data due to x being sorted in RAM.")
         }
       } else if (identical(substitute(on), as.name(".NATURAL"))) {
         naturaljoin = TRUE
@@ -686,10 +686,7 @@ replace_dot_alias = function(e) {
     if (is.null(jsub)) return(NULL)
 
     if (!with) {
-      if (jsub %iscall% ":=") {
-        # TODO(>=1.18.0): Simplify this error
-        stopf("with=FALSE together with := was deprecated in v1.9.4 released Oct 2014; this has been warning since v1.15.0. Please wrap the LHS of := with parentheses; e.g., DT[,(myVar):=sum(b),by=a] to assign to column name(s) held in variable myVar. See ?':=' for other examples.")
-      }
+      if (jsub %iscall% ":=") stopf("`:=` is only supported under with=TRUE, see ?`:=`.")
       # missingby was already checked above before dealing with i
       if (jsub %iscall% c("!", "-") && length(jsub)==2L) {  # length 2 to only match unary, #2109
         notj = TRUE

R/fdroplevels.R

@@ -8,8 +8,7 @@ fdroplevels = function(x, exclude = if (anyNA(levels(x))) NULL else NA, ...) {
   ans
 }
 
-droplevels.data.table = function(x, except=NULL, exclude, in.place=NULL, ...){
-  if (!is.null(in.place)) stopf("droplevels() with in.place=TRUE is deprecated. Use setdroplevels() instead.")
+droplevels.data.table = function(x, except=NULL, exclude, ...){
   x = copy(x)
   if (missing(exclude)) exclude = NULL
   setdroplevels(x, except, exclude)[]

R/fread.R

@@ -7,7 +7,7 @@ showProgress=getOption("datatable.showProgress",interactive()), data.table=getOp
 nThread=getDTthreads(verbose), logical01=getOption("datatable.logical01",FALSE),
 logicalYN=getOption("datatable.logicalYN", FALSE),
 keepLeadingZeros=getOption("datatable.keepLeadingZeros",FALSE),
-yaml=FALSE, autostart=NULL, tmpdir=tempdir(), tz="UTC")
+yaml=FALSE, tmpdir=tempdir(), tz="UTC")
 {
   if (missing(input)+is.null(file)+is.null(text)+is.null(cmd) < 3L) stopf("Used more than one of the arguments input=, file=, text= and cmd=.")
   input_has_vars = length(all.vars(substitute(input)))>0L  # see news for v1.11.6
@@ -124,7 +124,6 @@ yaml=FALSE, autostart=NULL, tmpdir=tempdir(), tz="UTC")
 
     input = file
   }
-  if (!is.null(autostart)) stopf("'autostart' is deprecated. Consider skip='string' or skip=n. This argument will be removed in the next release.");
   if (is.logical(colClasses)) {
     if (!allNA(colClasses)) stopf("colClasses is type 'logical' which is ok if all NA but it has some TRUE or FALSE values in it which is not allowed. Please consider the drop= or select= argument instead. See ?fread.")
     colClasses = NULL

R/fwrite.R

@@ -3,8 +3,7 @@ fwrite = function(x, file="", append=FALSE, quote="auto",
            sep2=c("","|",""), eol=if (.Platform$OS.type=="windows") "\r\n" else "\n",
            na="", dec=".", row.names=FALSE, col.names=TRUE,
            qmethod=c("double","escape"),
-           logical01=getOption("datatable.logical01", FALSE), # due to change to TRUE; see NEWS
-           logicalAsInt=NULL,
+           logical01=getOption("datatable.logical01", FALSE),
            scipen=getOption('scipen', 0L),
            dateTimeAs = c("ISO","squash","epoch","write.csv"),
            buffMB=8L, nThread=getDTthreads(verbose),
@@ -23,9 +22,6 @@ fwrite = function(x, file="", append=FALSE, quote="auto",
   compress = match.arg(compress)
   dateTimeAs = match.arg(dateTimeAs)
   dateTimeAs = chmatch(dateTimeAs, c("ISO", "squash", "epoch", "write.csv")) - 1L
-  if (!is.null(logicalAsInt)) {
-    stopf("logicalAsInt has been renamed logical01 for consistency with fread.")
-  }
   scipen = if (is.numeric(scipen)) as.integer(scipen) else 0L
   buffMB = as.integer(buffMB)
   nThread = as.integer(nThread)

R/onLoad.R

@@ -1,15 +1,8 @@
 # nocov start
 
-.pkg.store = new.env()
-.pkg.store$.unsafe.done = FALSE
 .unsafe.opt = function() {
-  if (.pkg.store$.unsafe.done) return(invisible())
-  val = getOption("datatable.nomatch")
-  if (is.null(val)) return(invisible())  # not defined (it hasn't been defined in .onLoad since v1.12.4)
-  warningf("Option 'datatable.nomatch' is defined but is now ignored. Please see note 11 in v1.12.4 NEWS (Oct 2019), and note 14 in v1.14.2.")
-  # leave this as warning for a long time
-  .pkg.store$.unsafe.done = TRUE
-  invisible()
+  if (!is.null(getOption("datatable.nomatch")))
+    stopf("Option 'datatable.nomatch' is defined but is now ignored. Please see note 11 in v1.12.4 NEWS (Oct 2019), and note 14 in v1.14.2.")
 }
 
 .Last.updated = vector("integer", 1L) # exported variable; number of rows updated by the last := or set(), #1885