Merge branch 'master' into deprec-autostart

ben-schwen · web-flow · commit cd8026da6ff2 · 2024-12-30T19:08:39.000+01:00
diff --git a/NAMESPACE b/NAMESPACE
@@ -5,7 +5,7 @@ importFrom(methods, "S3Part<-", slotNames)
 exportClasses(data.table, IDate, ITime)
 ##
 
-export(data.table, tables, setkey, setkeyv, key, "key<-", haskey, CJ, SJ, copy)
+export(data.table, tables, setkey, setkeyv, key, haskey, CJ, SJ, copy)
 export(rowwiseDT)
 export(setindex, setindexv, indices)
 export(as.data.table,is.data.table,test.data.table)
diff --git a/NEWS.md b/NEWS.md
@@ -137,7 +137,15 @@ rowwiseDT(
 
 6. `measurev()` was implemented and documented in v1.15.0, for use within `melt()`, and it is now exported (dependent packages can now use without a NOTE from CRAN check).
 
-7. Deprecation of `fread(autostart=)` has been upgraded to an error. It has been warning since v1.11.0 (6 years ago). The argument will be removed in the next release.
+7. The `dcast()` and `melt()` generics no longer attempt to redirect to {reshape2} methods when passed non-`data.table`s. If you're still using {reshape2}, you must use namespace-qualification: `reshape2::dcast()`, `reshape2::melt()`. We have been warning about the deprecation since v1.12.4 (2019). Please note that {reshape2} is retired.
+
+8. `showProgress` in `[` is disabled for "trivial" grouping (`.NGRP==1L`), [#6668](https://github.com/Rdatatable/data.table/issues/6668). Thanks @MichaelChirico for the request and @joshhwuu for the PR.
+
+9. `key<-`, marked as deprecated since 2012 and unusable since v1.15.0, has been fully removed.
+
+10. Deprecation of `logicalAsInt` argument to `fwrite()` has been upgraded from a warning (since v1.15.0) to an error. It will be removed in the next release.
+
+11. Deprecation of `fread(autostart=)` has been upgraded to an error. It has been warning since v1.11.0 (6 years ago). The argument will be removed in the next release.
 
 # data.table [v1.16.2](https://github.com/Rdatatable/data.table/milestone/35) (9 October 2024)
 
diff --git a/R/data.table.R b/R/data.table.R
@@ -459,9 +459,9 @@ replace_dot_alias = function(e) {
         if (!len_common_names) stopf("Attempting to do natural join but no common columns in provided tables")
         if (verbose) {
           which_cols_msg = if (len_common_names == length(x)) {
-            catf("Joining but 'x' has no key, natural join using all 'x' columns")
+            catf("Joining but 'x' has no key, natural join using all 'x' columns\n")
           } else {
-            catf("Joining but 'x' has no key, natural join using: %s", brackify(common_names))
+            catf("Joining but 'x' has no key, natural join using: %s\n", brackify(common_names))
           }
         }
         on = common_names
@@ -2077,8 +2077,6 @@ as.matrix.data.table = function(x, rownames=NULL, rownames.value=NULL, ...) {
   if (!is.null(rownames)) {
     if (!is.null(rownames.value)) stopf("rownames and rownames.value cannot both be used at the same time")
     if (length(rownames)>1L) {
-      # TODO in future as warned in NEWS for 1.11.6:
-      #   warningf("length(rownames)>1 is deprecated. Please use rownames.value= instead")
       if (length(rownames)!=nrow(x))
         stopf("length(rownames)==%d but nrow(DT)==%d. The rownames argument specifies a single column name or number. Consider rownames.value= instead.", length(rownames), nrow(x))
       rownames.value = rownames
diff --git a/R/fcast.R b/R/fcast.R
@@ -12,17 +12,10 @@ dcast <- function(
   data, formula, fun.aggregate = NULL, ..., margins = NULL,
   subset = NULL, fill = NULL, value.var = guess(data)
 ) {
-  if (is.data.table(data)) UseMethod("dcast", data)
-  # nocov start
-  else {
-    data_name = deparse(substitute(data))
-    ns = tryCatch(getNamespace("reshape2"), error=function(e)
-      stopf("The %1$s generic in data.table has been passed a %2$s, but data.table::%1$s currently only has a method for data.tables. Please confirm your input is a data.table, with setDT(%3$s) or as.data.table(%3$s). If you intend to use a method from reshape2, try installing that package first, but do note that reshape2 is superseded and is no longer actively developed.", "dcast", class1(data), data_name))
-    warningf("The %1$s generic in data.table has been passed a %2$s and will attempt to redirect to the relevant reshape2 method; please note that reshape2 is superseded and is no longer actively developed, and this redirection is now deprecated. Please do this redirection yourself like reshape2::%1$s(%3$s). In the next version, this warning will become an error.", "dcast", class1(data), data_name)
-    ns$dcast(data, formula, fun.aggregate = fun.aggregate, ..., margins = margins,
-             subset = subset, fill = fill, value.var = value.var)
-  }
-  # nocov end
+  # TODO(>=1.19.0): Remove this, just let dispatch to 'default' method fail.
+  if (!is.data.table(data))
+    stopf("The %1$s generic in data.table has been passed a %2$s, but data.table::%1$s currently only has a method for data.tables. Please confirm your input is a data.table, with setDT(%3$s) or as.data.table(%3$s). If you intend to use a method from reshape2, try installing that package first, but do note that reshape2 is superseded and is no longer actively developed.", "dcast", class1(data), deparse(substitute(data))) # nocov
+  UseMethod("dcast", data)
 }
 
 check_formula = function(formula, varnames, valnames, value.var.in.LHSdots, value.var.in.RHSdots) {
diff --git a/R/fmelt.R b/R/fmelt.R
@@ -1,22 +1,15 @@
 # reshape2 dependency was originally abandoned because (1) we wanted to be in control
 #   of the R version dependency and (2) reshape2::dcast is not generic.
-#   reshape2 package is deprecated since December 2017, so we'll deprecate our
+#   reshape2 package is deprecated since December 2017, so we've deprecated our
 #   redirection as well
 
 melt = function(data, ..., na.rm = FALSE, value.name = "value") {
   UseMethod("melt", data)
 }
 
+# TODO(>=1.19.0): Remove this, just let dispatch to 'default' method fail.
 melt.default = function(data, ..., na.rm = FALSE, value.name = "value") {
-  # if no registered method exists for data, attempts to redirect data to reshape2::melt;
-  # CRAN package edarf and others fail without the redirection
-  # nocov start
-  data_name = deparse(substitute(data))
-  ns = tryCatch(getNamespace("reshape2"), error=function(e)
-    stopf("The %1$s generic in data.table has been passed a %2$s, but data.table::%1$s currently only has a method for data.tables. Please confirm your input is a data.table, with setDT(%3$s) or as.data.table(%3$s). If you intend to use a method from reshape2, try installing that package first, but do note that reshape2 is superseded and is no longer actively developed.", "melt", class1(data), data_name))
-  warningf("The %1$s generic in data.table has been passed a %2$s and will attempt to redirect to the relevant reshape2 method; please note that reshape2 is superseded and is no longer actively developed, and this redirection is now deprecated. To continue using melt methods from reshape2 while both packages are attached, e.g. melt.list, you can prepend the namespace, i.e. reshape2::%1$s(%3$s). In the next version, this warning will become an error.", "melt", class1(data), data_name)
-  ns$melt(data, ..., na.rm=na.rm, value.name=value.name)
-  # nocov end
+  stopf("The %1$s generic in data.table has been passed a %2$s and will attempt to redirect to the relevant reshape2 method; please note that reshape2 is superseded and is no longer actively developed, and this redirection is now deprecated. To continue using melt methods from reshape2 while both packages are attached, e.g. melt.list, you can prepend the namespace, i.e. reshape2::%1$s(%3$s). In the next version, this warning will become an error.", "melt", class1(data), deparse(substitute(data))) # nocov
 }
 
 patterns = function(..., cols=character(0L), ignore.case=FALSE, perl=FALSE, fixed=FALSE, useBytes=FALSE) {
diff --git a/R/fwrite.R b/R/fwrite.R
@@ -4,7 +4,7 @@ fwrite = function(x, file="", append=FALSE, quote="auto",
            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=logical01,
+           logicalAsInt=NULL,
            scipen=getOption('scipen', 0L),
            dateTimeAs = c("ISO","squash","epoch","write.csv"),
            buffMB=8, nThread=getDTthreads(verbose),
@@ -24,12 +24,8 @@ fwrite = function(x, file="", append=FALSE, quote="auto",
   else if (length(dateTimeAs)>1L) stopf("dateTimeAs must be a single string")
   dateTimeAs = chmatch(dateTimeAs, c("ISO","squash","epoch","write.csv"))-1L
   if (is.na(dateTimeAs)) stopf("dateTimeAs must be 'ISO','squash','epoch' or 'write.csv'")
-  if (!missing(logical01) && !missing(logicalAsInt))
-    stopf("logicalAsInt has been renamed logical01. Use logical01 only, not both.")
-  if (!missing(logicalAsInt)) {
-    warningf("logicalAsInt has been renamed logical01 for consistency with fread. It works fine for now but please change to logical01 at your convenience so we can remove logicalAsInt in future.")
-    logical01 = logicalAsInt
-    logicalAsInt=NULL
+  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)
diff --git a/R/setkey.R b/R/setkey.R
@@ -18,11 +18,6 @@ setindexv = function(x, cols, verbose=getOption("datatable.verbose")) {
   }
 }
 
-# Has been warning since 2012, with stronger warning in Mar 2019 (note in news for 1.12.2); #3399
-"key<-" = function(x,value) {
-  stopf("key(x)<-value is deprecated and not supported. Please change to use setkey() with perhaps copy(). Has been warning since 2012.")
-}
-
 setkeyv = function(x, cols, verbose=getOption("datatable.verbose"), physical=TRUE)
 {
   if (is.null(cols)) {   # this is done on a data.frame when !cedta at top of [.data.table
diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw
@@ -1645,11 +1645,7 @@ test(504, dtB$k, 1:4)  # or dtB
 DT = data.table(a=1:2,b=1:4,key="a")
 test(505, DT[J(a=1,b=6),sum(i.b*b),by=.EACHI]$V1, 24)  # 24 now 'double' because i.b is 'double'
 
-# Test := after a key<-
-DT = data.table(a=3:1,b=4:6)
-test(506, key(DT)<-"a", error="deprecated")
-
-# tests 508, 509, 510 related to follow-up operations after key<-, which are now irrelevant
+# tests 506, 508, 509, 510 related to follow-up operations after key<-, which are now irrelevant
 
 # Test new functions chmatch and %chin%
 y=letters
@@ -10937,10 +10933,7 @@ test(1736.03, fwrite(DT, sep2=c("",",","")), error="sep.*,.*sep2.*,.*must all be
 test(1736.04, fwrite(DT, sep2=c("","||","")), error="nchar.*sep2.*2")
 test(1736.05, capture.output(fwrite(DT, sep='|', sep2=c("c(",",",")"), logical01=FALSE)), c("A|B|C", "1|c(1,2,3,4,5,6,7,8,9,10)|c(s,t,u,v,w)",
  "2|c(15,16,17,18)|c(1.2,2.3,3.4,3.14159265358979,-9)", "3|c(7)|c(foo,bar)", "4|c(9,10)|c(TRUE,TRUE,FALSE)"))
-test(1736.06, capture.output(fwrite(DT, sep='|', sep2=c("{",",","}"), logicalAsInt=TRUE)),
- c("A|B|C", "1|{1,2,3,4,5,6,7,8,9,10}|{s,t,u,v,w}",
- "2|{15,16,17,18}|{1.2,2.3,3.4,3.14159265358979,-9}", "3|{7}|{foo,bar}", "4|{9,10}|{1,1,0}"),
- warning="logicalAsInt has been renamed logical01")
+test(1736.06, fwrite(DT, sep='|', sep2=c("{",",","}"), logicalAsInt=TRUE), error="logicalAsInt has been renamed logical01")
 DT = data.table(A=c("foo","ba|r","baz"))
 test(1736.07, capture.output(fwrite(DT,na="")), c("A","foo","ba|r","baz"))    # no list column so no need to quote
 test(1736.08, capture.output(fwrite(DT)), c("A","foo","ba|r","baz"))
@@ -15961,8 +15954,7 @@ DT[ , z := 0L]
 test(2074.31, dcast(DT, V1 ~ z, fun.aggregate=eval(quote(length)), value.var='z'),
      data.table(V1=c('a', 'b'), `0`=2:1,key='V1'))
 
-# fwrite both logical args
-test(2074.32, fwrite(DT, logical01=TRUE, logicalAsInt=TRUE), error="logicalAsInt has been renamed")
+# 2074.32 tested that setting both logical01 and logicalAsInt errored; no longer relevant
 
 # merge.data.table
 test(2074.33, merge(DT, DT, by.x = 1i, by.y=1i), error="A non-empty vector of column names is required")
diff --git a/man/deprecated.Rd b/man/deprecated.Rd
diff --git a/man/fread.Rd b/man/fread.Rd
@@ -22,7 +22,7 @@ key=NULL, index=NULL,
 showProgress=getOption("datatable.showProgress", interactive()),
 data.table=getOption("datatable.fread.datatable", TRUE),
 nThread=getDTthreads(verbose),
-logical01=getOption("datatable.logical01", FALSE),  # due to change to TRUE; see NEWS
+logical01=getOption("datatable.logical01", FALSE),
 logicalYN=getOption("datatable.logicalYN", FALSE),
 keepLeadingZeros = getOption("datatable.keepLeadingZeros", FALSE),
 yaml=FALSE, autostart=NULL, tmpdir=tempdir(), tz="UTC"
diff --git a/man/fwrite.Rd b/man/fwrite.Rd
@@ -12,7 +12,7 @@ fwrite(x, file = "", append = FALSE, quote = "auto",
   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 = logical01,  # deprecated
+  logicalAsInt = NULL,  # deprecated
   scipen = getOption('scipen', 0L),
   dateTimeAs = c("ISO","squash","epoch","write.csv"),
   buffMB = 8L, nThread = getDTthreads(verbose),
diff --git a/man/setkey.Rd b/man/setkey.Rd
@@ -142,6 +142,11 @@ setindex(DT, B)
 indices(DT)           # get indices single vector
 indices(DT, vectors = TRUE) # get indices list
 
+# Setting multiple indices at once
+DT = data.table(A = 5:1, B = letters[5:1], C = 10:6)
+setindexv(DT, list(c("A", "B"), c("B", "C")))
+print(DT, show.indices=TRUE)
+
 # Use the dot .(subset_value) syntax with integer keys:
 DT = data.table(id = 2:1)
 setkey(DT, id)
diff --git a/src/assign.c b/src/assign.c
@@ -33,7 +33,7 @@ void setselfref(SEXP x) {
     R_NilValue,                  // for identical() to return TRUE. identical() doesn't look at tag and prot
     PROTECT(getAttrib(x, R_NamesSymbol)),  // to detect if names has been replaced and its tl lost, e.g. setattr(DT,"names",...)
     PROTECT(R_MakeExternalPtr(   // to avoid an infinite loop in object.size(), if prot=x here
-      x,                         // to know if this data.table has been copied by key<-, attr<-, names<-, etc.
+      x,                         // to know if this data.table has been copied by attr<-, names<-, etc.
       R_NilValue,                // this tag and prot currently unused
       R_NilValue
     ))
diff --git a/src/chmatch.c b/src/chmatch.c
@@ -35,12 +35,13 @@ static SEXP chmatchMain(SEXP x, SEXP table, int nomatch, bool chin, bool chmatch
     return ans;
   }
   // Since non-ASCII strings may be marked with different encodings, it only make sense to compare
-  // the bytes under a same encoding (UTF-8) #3844 #3850
+  // the bytes under a same encoding (UTF-8) #3844 #3850.
+  // Not 'const' because we might SET_TRUELENGTH() below.
   SEXP *xd;
   if (isSymbol(x)) {
     xd = &sym;
   } else {
-    xd = STRING_PTR_RO(PROTECT(coerceUtf8IfNeeded(x))); nprotect++;
+    xd = (SEXP *)STRING_PTR_RO(PROTECT(coerceUtf8IfNeeded(x))); nprotect++;
   }
   const SEXP *td = STRING_PTR_RO(PROTECT(coerceUtf8IfNeeded(table))); nprotect++;
   if (xlen==1) {
diff --git a/src/coalesce.c b/src/coalesce.c
@@ -53,7 +53,7 @@ SEXP coalesce(SEXP x, SEXP inplaceArg) {
     first = PROTECT(copyAsPlain(first)); nprotect++;
     if (verbose) Rprintf(_("coalesce copied first item (inplace=FALSE)\n"));
   }
-  void **valP = (void **)R_alloc(nval, sizeof(void *));
+  const void **valP = (const void **)R_alloc(nval, sizeof(void *));
   switch(TYPEOF(first)) {
   case LGLSXP:
   case INTSXP: {
@@ -66,7 +66,7 @@ SEXP coalesce(SEXP x, SEXP inplaceArg) {
         finalVal = tt;
         break;  // stop early on the first singleton that is not NA; minimizes deepest loop body below
       }
-      valP[k++] = INTEGER(item);
+      valP[k++] = INTEGER_RO(item);
     }
     const bool final=(finalVal!=NA_INTEGER);
     #pragma omp parallel for num_threads(getDTthreads(nrow, true))
@@ -89,7 +89,7 @@ SEXP coalesce(SEXP x, SEXP inplaceArg) {
           finalVal = tt;
           break;
         }
-        valP[k++] = REAL(item);
+        valP[k++] = REAL_RO(item);
       }
       const bool final = (finalVal!=NA_INTEGER64);
       #pragma omp parallel for num_threads(getDTthreads(nrow, true))
@@ -110,7 +110,7 @@ SEXP coalesce(SEXP x, SEXP inplaceArg) {
           finalVal = tt;
           break;
         }
-        valP[k++] = REAL(item);
+        valP[k++] = REAL_RO(item);
       }
       const bool final = !ISNAN(finalVal);
       #pragma omp parallel for num_threads(getDTthreads(nrow, true))
@@ -133,7 +133,7 @@ SEXP coalesce(SEXP x, SEXP inplaceArg) {
         finalVal = tt;
         break;
       }
-      valP[k++] = COMPLEX(item);
+      valP[k++] = COMPLEX_RO(item);
     }
     const bool final = !ISNAN(finalVal.r) && !ISNAN(finalVal.i);
     #pragma omp parallel for num_threads(getDTthreads(nrow, true))
diff --git a/src/data.table.h b/src/data.table.h
@@ -6,6 +6,10 @@
 #  define ALTREP(x) 0     // #2866
 #  define USE_RINTERNALS  // #3301
 #  define DATAPTR_RO(x) ((const void *)DATAPTR(x))
+#  define STRING_PTR_RO STRING_PTR
+#  define INTEGER_RO INTEGER
+#  define REAL_RO REAL
+#  define COMPLEX_RO COMPLEX
 #  define R_Calloc(x, y) Calloc(x, y)         // #6380
 #  define R_Realloc(x, y, z) Realloc(x, y, z)
 #  define R_Free(x) Free(x)
@@ -15,9 +19,6 @@
 #endif
 #include <Rinternals.h>
 #define SEXPPTR_RO(x) ((const SEXP *)DATAPTR_RO(x))  // to avoid overhead of looped STRING_ELT and VECTOR_ELT
-#ifndef STRING_PTR_RO
-#define STRING_PTR_RO STRING_PTR
-#endif
 #include <stdint.h>    // for uint64_t rather than unsigned long long
 #include <stdarg.h>    // for va_list, va_start
 #include <stdbool.h>
diff --git a/src/dogroups.c b/src/dogroups.c
@@ -70,10 +70,7 @@ SEXP dogroups(SEXP dt, SEXP dtcols, SEXP groups, SEXP grpcols, SEXP jiscols, SEX
   SEXP ans=NULL, jval, thiscol, BY, N, I, GRP, iSD, xSD, rownames, s, RHS, target, source;
   Rboolean wasvector, firstalloc=FALSE, NullWarnDone=FALSE;
   const bool verbose = LOGICAL(verboseArg)[0]==1;
-  const bool showProgress = LOGICAL(showProgressArg)[0]==1;
   double tstart=0, tblock[10]={0}; int nblock[10]={0}; // For verbose printing, tstart is updated each block
-  double startTime = (showProgress) ? wallclock() : 0; // For progress printing, startTime is set at the beginning
-  double nextTime = (showProgress) ? startTime+3 : 0; // wait 3 seconds before printing progress
   bool hasPrinted = false;
 
   if (!isInteger(order)) internal_error(__func__, "order not integer vector"); // # nocov
@@ -89,6 +86,10 @@ SEXP dogroups(SEXP dt, SEXP dtcols, SEXP groups, SEXP grpcols, SEXP jiscols, SEX
   // fix for longstanding FR/bug, #495. E.g., DT[, c(sum(v1), lapply(.SD, mean)), by=grp, .SDcols=v2:v3] resulted in error.. the idea is, 1) we create .SDall, which is normally == .SD. But if extra vars are detected in jexp other than .SD, then .SD becomes a shallow copy of .SDall with only .SDcols in .SD. Since internally, we don't make a copy, changing .SDall will reflect in .SD. Hopefully this'll workout :-).
   SEXP SDall = PROTECT(findVar(install(".SDall"), env)); nprotect++;  // PROTECT for rchk
   SEXP SD = PROTECT(findVar(install(".SD"), env)); nprotect++;
+  
+  const bool showProgress = LOGICAL(showProgressArg)[0]==1 && ngrp > 1; // showProgress only if more than 1 group
+  double startTime = (showProgress) ? wallclock() : 0; // For progress printing, startTime is set at the beginning
+  double nextTime = (showProgress) ? startTime+3 : 0; // wait 3 seconds before printing progress
 
   defineVar(sym_BY, BY = PROTECT(allocVector(VECSXP, ngrpcols)), env); nprotect++;  // PROTECT for rchk
   SEXP bynames = PROTECT(allocVector(STRSXP, ngrpcols));  nprotect++;   // TO DO: do we really need bynames, can we assign names afterwards in one step?
diff --git a/vignettes/datatable-joins.Rmd b/vignettes/datatable-joins.Rmd

Original file line number	Diff line number	Diff line change
`@@ -18,11 +18,6 @@ setindexv = function(x, cols, verbose=getOption("datatable.verbose")) {`
`18`	`18`	`}`
`19`	`19`	`}`
`20`	`20`
`21`		`-# Has been warning since 2012, with stronger warning in Mar 2019 (note in news for 1.12.2); #3399`
`22`		`-"key<-" = function(x,value) {`
`23`		`- stopf("key(x)<-value is deprecated and not supported. Please change to use setkey() with perhaps copy(). Has been warning since 2012.")`
`24`		`-}`
`25`		`-`
`26`	`21`	`setkeyv = function(x, cols, verbose=getOption("datatable.verbose"), physical=TRUE)`
`27`	`22`	`{`
`28`	`23`	`if (is.null(cols)) { # this is done on a data.frame when !cedta at top of [.data.table`