Merge branch 'master' into parserFunctionRename

Author: badasahog

GOVERNANCE.md

@@ -1,4 +1,4 @@
-Governance for the R data.table project
+# Governance for the R data.table project
 
 # Purpose and scope
 
@@ -121,7 +121,7 @@ Funds acquired by the data.table project will be disbursed at the discretion of
 
 # Code of conduct
 
-The full Code of Conduct can be found [here](CODE_OF_CONDUCT.md), including details for reporting violations.
+The full Code of Conduct can be found [here](.github/CODE_OF_CONDUCT.md), including details for reporting violations.
 
 ## Reporting Responsibility
 

NEWS.md

@@ -8,6 +8,10 @@
 
 1. `data.table(x=1, <expr>)`, where `<expr>` is an expression resulting in a 1-column matrix without column names, will eventually have names `x` and `V2`, not `x` and `V1`, consistent with `data.table(x=1, <expr>)` where `<expr>` results in an atomic vector, for example `data.table(x=1, cbind(1))` and `data.table(x=1, 1)` will both have columns named `x` and `V2`. In this release, the matrix case continues to be named `V1`, but the new behavior can be activated by setting `options(datatable.old.matrix.autoname)` to `FALSE`. See point 5 under Bug Fixes for more context; this change will provide more internal consistency as well as more consistency with `data.frame()`.
 
+### BREAKING CHANGE
+
+1. `dcast()` now errors when `fun.aggregate` returns length != 1 (consistent with documentation), regardless of `fill`, [#6629](https://github.com/Rdatatable/data.table/issues/6629). Previously, when `fill` was not `NULL`, `dcast` warned and returned an undefined result. This change has been planned since 1.16.0 (25 Aug 2024).
+
 ### NEW FEATURES
 
 1. New `sort_by()` method for data.tables, [#6662](https://github.com/Rdatatable/data.table/issues/6662). It uses `forder()` to improve upon the data.frame method and also matches `DT[order(...)]` behavior with respect to locale. Thanks @rikivillalba for the suggestion and PR.
@@ -107,6 +111,8 @@
 
 18. `fwrite` now allows `dec` to be the same as `sep` for edge cases where only one will be written, e.g. 0-row or 1-column tables. [#7227](https://github.com/Rdatatable/data.table/issues/7227). Thanks @MichaelChirico for the report and @venom1204 for the fix.
 
+19. Ellipsis elements like `..1` are correctly excluded when searching for variables in "up-a-level" syntax inside `[`, [#5460](https://github.com/Rdatatable/data.table/issues/5460). Thanks @ggrothendieck for the report and @MichaelChirico for the fix.
+
 ### NOTES
 
 1. The following in-progress deprecations have proceeded:

R/data.table.R

@@ -264,7 +264,8 @@ replace_dot_alias = function(e) {
   if (!missing(j)) {
     jsub = replace_dot_alias(jsub)
     root = root_name(jsub)
-    av = all.vars(jsub)
+    # exclude '..1' etc. for #5460
+    av = grepv("^[.][.](?:[.]|[0-9]+)$", all.vars(jsub), invert=TRUE)
     all..names = FALSE
     if ((.is_withFALSE_range(jsub, x, root, av)) ||
         (root %chin% c("-","!") && jsub[[2L]] %iscall% '(' && jsub[[2L]][[2L]] %iscall% ':') || ## x[, !(V8:V10)]
@@ -1297,8 +1298,8 @@ replace_dot_alias = function(e) {
   SDenv = new.env(parent=parent.frame())
 
   syms = all.vars(jsub)
-  syms = syms[ startsWith(syms, "..") ]
-  syms = syms[ substr(syms, 3L, 3L) != "." ]  # exclude ellipsis
+  syms = syms[startsWith(syms, "..")]
+  syms = grepv("^[.][.](?:[.]|[0-9]+)$", syms, invert=TRUE) # exclude ellipsis and '..n' ellipsis elements
   for (sym in syms) {
     if (sym %chin% names_x) {
       # if "..x" exists as column name, use column, for backwards compatibility; e.g. package socialmixr in rev dep checks #2779

R/fcast.R

@@ -186,11 +186,7 @@ dcast.data.table = function(data, formula, fun.aggregate = NULL, sep = "_", ...,
     maybe_err = function(list.of.columns) {
       if (!all(lengths(list.of.columns) == 1L)) {
         msg = gettext("Aggregating functions should take a vector as input and return a single value (length=1), but they do not, so the result is undefined. Please fix by modifying your function so that a single value is always returned.")
-        if (is.null(fill)) { # TODO change to always stopf #6329
-          stop(msg, domain=NA, call. = FALSE)
-        } else {
-          warning(msg, domain=NA, call. = FALSE)
-        }
+        stop(msg, domain=NA, call. = FALSE)
       }
       list.of.columns
     }

inst/tests/tests.Rraw

@@ -13701,6 +13701,10 @@ test(1962.054, forder(DT, ), 3:1)
 
 test(1962.055, fsort(as.double(DT$a), internal = TRUE),
      error = 'Internal code should not be being called on type double')
+# coverage for fsort #7239
+set.seed(1)
+x = as.double(sample(0:(2^20), size = 1e6, replace = TRUE))
+test(1962.0551, sort(x), fsort(x))
 
 l = as.list(DT)
 test(1962.056, setorder(l, a), error = 'x must be a data.frame or data.table')
@@ -21564,16 +21568,16 @@ test(2333, as.expression(data.table(a = 1))[["a"]], 1)
 
 # regression test for hexdigits subscript overrun (uint8_t wraps over 255, unsigned overflow is well defined in c)
 local({
-	f = tempfile()
-	on.exit(unlink(f))
-	# the line is likely invalid in current encoding, so disable any translation, #7209
-	# test.data.table() sets options(encoding="UTF-8"), so go the long way around.
-	ff = file(f, encoding = "")
-	tryCatch(
-		writeLines(c('a', rep('0x1.ffffp0', 10000L), `Encoding<-`('0x1.ff\x9fp0', 'bytes'), rep('0x1.ffffp0', 20000L)), ff),
-		finally = close(ff)
-	)
-	test(2334, names(fread(f)), "a")
+  f = tempfile()
+  on.exit(unlink(f))
+  # the line is likely invalid in current encoding, so disable any translation, #7209
+  # test.data.table() sets options(encoding="UTF-8"), so go the long way around.
+  ff = file(f, encoding = "")
+  tryCatch(
+    writeLines(c('a', rep('0x1.ffffp0', 10000L), `Encoding<-`('0x1.ff\x9fp0', 'bytes'), rep('0x1.ffffp0', 20000L)), ff),
+    finally = close(ff)
+  )
+  test(2334, names(fread(f)), "a")
 })
 
 # Tests for new isoyear() helper (complement to isoweek) #7154
@@ -21620,3 +21624,34 @@ local({
   test(2338.9, {fwrite(dd, f, forceDecimal=FALSE); fread(f)}, di)
 })
 
+# '..1' is ...elt(1), not a variable named '1', when doing "up-a-level" search (#5460)
+DT = data.table(a=1.0)
+sqrt_dot_sym = function(...) sqrt(..1)
+test(2339.01, lapply(DT, function(...) sqrt(..1)), list(a=1.0))
+test(2339.02, lapply(DT, sqrt_dot_sym), list(a=1.0))
+test(2339.03, DT[, lapply(.SD, function(...) sqrt(..1))], data.table(a=1.0))
+test(2339.04, DT[, lapply(.SD, sqrt_dot_sym)], data.table(a=1.0))
+sqrt_elt_sym = function(...) sqrt(...elt(1))
+# TODO(R>=3.5.0): run this unconditionally
+if (!inherits(tryCatch(sqrt_elt_sym(1), error=identity), "error")) {
+  test(2339.05, lapply(DT, sqrt_elt_sym), list(a=1.0))
+  test(2339.06, lapply(DT, function(...) sqrt(...elt(1L))), list(a=1.0))
+  test(2339.07, DT[, lapply(.SD, sqrt_elt_sym)], data.table(a=1.0))
+  test(2339.08, DT[, lapply(.SD, function(...) sqrt(...elt(1L)))], data.table(a=1.0))
+}
+# edge case of weird, though technically valid, names:
+#  _not_ '..N' style and distinct from '...'
+#  `123`='a'; DT[, ..123] doesn't work, but that's fine & easy to work around
+DT = data.table(a = 4L)
+local({
+  `.123` = 'a'
+  test(2339.09, DT[, ...123], DT)
+})
+local({
+  ..123 = 'a'
+  test(2339.10, DT[, ....123], DT)
+})
+local({
+  ...123 = 'a'
+  test(2339.11, DT[, .....123], DT)
+})

man/fread.Rd

@@ -118,6 +118,15 @@ Currently, the \code{yaml} setting is somewhat inflexible with respect to incorp
 
 When \code{input} begins with http://, https://, ftp://, ftps://, or file://, \code{fread} detects this and \emph{downloads} the target to a temporary file (at \code{tempfile()}) before proceeding to read the file as usual. URLS (ftps:// and https:// as well as ftp:// and http://) paths are downloaded with \code{download.file} and \code{method} set to \code{getOption("download.file.method")}, defaulting to \code{"auto"}; and file:// is downloaded with \code{download.file} with \code{method="internal"}. NB: this implies that for file://, even files found on the current machine will be "downloaded" (i.e., hard-copied) to a temporary file. See \code{\link{download.file}} for more details.
 
+\bold{Automatic Decompression:}
+
+In many cases, \code{fread} can automatically detect and decompress files with common compression extensions directly, without needing an explicit connection object or shell commands. This works by checking the file extension.
+
+  \itemize{
+    \item \code{.gz} and \code{.bz2} are supported out of the box.
+    \item \code{.zip} is also supported. If the archive contains a single data file, \code{fread} will read it. If the archive contains multiple files, \code{fread} will produce an error.
+  }
+
 \bold{Shell commands:}
 
 \code{fread} accepts shell commands for convenience. The input command is run and its output written to a file in \code{tmpdir} (\code{\link{tempdir}()} by default) to which \code{fread} is applied "as normal". The details are platform dependent -- \code{system} is used on UNIX environments, \code{shell} otherwise; see \code{\link[base]{system}}.

src/fsort.c

@@ -1,6 +1,6 @@
 #include "data.table.h"
 
-#define INSERT_THRESH 200  // TODO: expose via api and test
+static const int INSERT_THRESH = 200;  // TODO: expose via api and test
 
 static void dinsert(double *x, const int n) {   // TODO: if and when twiddled, double => ull
   if (n<2) return;
@@ -43,17 +43,16 @@ static void dradix_r(  // single-threaded recursive worker
     return;
   }
 
-  uint64_t cumSum=0;
-  for (uint64_t i=0; cumSum<n; ++i) { // cumSum<n better than i<width as may return early
-    uint64_t tmp;
-    if ((tmp=counts[i])) {  // don't cumulate through 0s, important below to save a wasteful memset to zero
+  for (uint64_t i = 0, cumSum = 0; cumSum < n; i++) { // cumSum<n better than i<width as may return early
+    uint64_t tmp = counts[i];
+    if (tmp) {  // don't cumulate through 0s, important below to save a wasteful memset to zero
       counts[i] = cumSum;
       cumSum += tmp;
     }
   } // leaves cumSum==n && 0<i && i<=width
 
   tmp=in;
-  for (uint64_t i=0; i<n; ++i) {  // go forwards not backwards to give cpu pipeline better chance
+  for (uint64_t i = 0; i<n; ++i) {  // go forwards not backwards to give cpu pipeline better chance
     int thisx = (*(uint64_t *)tmp - minULL) >> fromBit & mask;
     working[ counts[thisx]++ ] = *tmp;
     tmp++;
@@ -71,8 +70,7 @@ static void dradix_r(  // single-threaded recursive worker
     return;
   }
 
-  cumSum=0;
-  for (int i=0; cumSum<n; ++i) {   // again, cumSum<n better than i<width as it can return early
+  for (uint64_t i = 0, cumSum = 0; cumSum < n; i++) {   // again, cumSum<n better than i<width as it can return early
     if (counts[i] == 0) continue;
     uint64_t thisN = counts[i] - cumSum;  // undo cummulate; i.e. diff
     if (thisN <= INSERT_THRESH) {