Merge branch 'master' into error-enhance

Author: Divendra2006

.ci/.lintr.R

@@ -9,10 +9,10 @@ linters = c(dt_linters, all_linters(
   packages = "lintr", # TODO(lintr->3.2.0): Remove this.
   # eq_assignment_linter(),
   brace_linter(allow_single_line = TRUE),
+  implicit_integer_linter(allow_colon = TRUE),
   # TODO(michaelchirico): Activate these incrementally. These are the
   #   parameterizations that match our style guide.
   # implicit_assignment_linter(allow_lazy = TRUE, allow_scoped = TRUE),
-  # implicit_integer_linter(allow_colon = TRUE),
   # system_time_linter = undesirable_function_linter(c(
   #   system.time = "Only run timings in benchmark.Rraw"
   # )),
@@ -85,6 +85,7 @@ exclusions = c(local({
       undesirable_function_linter = Inf
     )),
     exclusion_for_dir(c("vignettes", "vignettes/fr"), list(
+      implicit_integer_linter = Inf,
       quotes_linter = Inf,
       sample_int_linter = Inf
       # strings_as_factors_linter = Inf

.ci/linters/c/alloc_linter.R

@@ -4,8 +4,9 @@
 #   2. Check the next line for a check like 'if (!x || !y)'
 alloc_linter = function(c_obj) {
   lines = c_obj$lines
-  # Be a bit more precise to avoid mentions in comments
-  alloc_lines = grep(R"{=\s*([(]\w+\s*[*][)])?[mc]alloc[(]}", lines)
+  # Be a bit more precise to avoid mentions in comments, and allow
+  #   malloc(0) to be used for convenience (e.g. #6757)
+  alloc_lines = grep(R"{=\s*([(]\w+\s*[*][)])?[mc]alloc[(][^0]}", lines)
   if (!length(alloc_lines)) return()
   # int *tmp=(int*)malloc(...); or just int tmp=malloc(...);
   alloc_keys = lines[alloc_lines] |>

.dev/CRAN_Release.cmd

@@ -114,12 +114,6 @@ grep -P "\t" ./src/*.c
 grep -n "[^A-Za-z0-9]T[^A-Za-z0-9]" ./inst/tests/tests.Rraw
 grep -n "[^A-Za-z0-9]F[^A-Za-z0-9]" ./inst/tests/tests.Rraw
 
-# All integers internally should have L suffix to avoid lots of one-item coercions
-# Where 0 numeric is intended we should perhaps use 0.0 for clarity and make the grep easier
-# 1) tolerance=0 usages in setops.R are valid numeric 0, as are anything in strings
-# 2) leave the rollends default using roll>=0 though; comments in PR #3803
-grep -Enr "^[^#]*(?:\[|==|>|<|>=|<=|,|\(|\+)\s*[-]?[0-9]+[^0-9L:.e]" R | grep -Ev "stop|warning|tolerance"
-
 # Never use ifelse. fifelse for vectors when necessary (nothing yet)
 grep -Enr "\bifelse" R
 
@@ -135,10 +129,6 @@ grep -Fn "tryCatch" ./inst/tests/*.Rraw
 # All % in *.Rd should be escaped otherwise text gets silently chopped
 grep -n "[^\]%" ./man/*.Rd
 
-# if (a & b) is either invalid or inefficient (ditto for replace & with |);
-#   if(any(a [&|] b)) is appropriate b/c of collapsing the logical vector to scalar
-grep -nr "^[^#]*if[^&#]*[^&#\"][&][^&]" R | grep -Ev "if\s*[(](?:any|all)"
-
 # seal leak potential where two unprotected API calls are passed to the same
 # function call, usually involving install() or mkChar()
 # Greppable thanks to single lines and wide screens

NEWS.md

@@ -69,7 +69,7 @@ rowwiseDT(
 
 6. `fread()` gains `logicalYN` argument to read columns consisting only of strings `Y`, `N` as `logical` (as opposed to character), [#4563](https://github.com/Rdatatable/data.table/issues/4563). The default is controlled by option `datatable.logicalYN`, itself defaulting to `FALSE`, for back-compatibility -- some smaller tables (especially sharded tables) might inadvertently read a "true" string column as `logical` and cause bugs. This is particularly important for tables with a column named `y` or `n` -- automatic header detection under `logicalYN=TRUE` will see these values in the first row as being "data" as opposed to column names. A parallel option was not included for `fwrite()` at this time -- users looking for a compact representation of logical columns can still use `fwrite(logical01=TRUE)`. We also opted for now to check only `Y`, `N` and not `Yes`/`No`/`YES`/`NO`.
 
-7. `fwrite()` with `compress="gzip"` produces compatible gz files when composed of multiple independent chunks owing to parallelization, [#6356](https://github.com/Rdatatable/data.table/issues/6356).  Earlier `fwrite()` versions could have issues with HTTP upload using `Content-Encoding: gzip` and `Transfer-Encoding: chunked`. Thanks to @oliverfoster for report and @philippechataignon for the fix.
+7. `fwrite()` with `compress="gzip"` produces compatible gz files when composed of multiple independent chunks owing to parallelization, [#6356](https://github.com/Rdatatable/data.table/issues/6356).  Earlier `fwrite()` versions could have issues with HTTP upload using `Content-Encoding: gzip` and `Transfer-Encoding: chunked`. Thanks to @oliverfoster for report and @philippechataignon for the fix. Thanks also @aitap for pre-release testing that found some possible memory leaks in the initial fix.
 
 8. `fwrite()` gains a new parameter `compressLevel` to control compression level for gzip, [#5506](https://github.com/Rdatatable/data.table/issues/5506). This parameter balances compression speed and total compression, and corresponds directly to the analogous command-line parameter, e.g. `compressLevel=4` corresponds to passing `-4`; the default, `6`, matches the command-line default, i.e. equivalent to passing `-6`. Thanks @mgarbuzov for the request and @philippechataignon for implementing.
 

R/IDateTime.R

@@ -186,7 +186,7 @@ as.ITime.POSIXlt = function(x, ms = 'truncate', ...) {
 }
 
 as.ITime.times = function(x, ms = 'truncate', ...) {
-  secs = 86400 * (unclass(x) %% 1)
+  secs = 86400L * (unclass(x) %% 1L)
   secs = clip_msec(secs, ms)
   (setattr(secs, "class", "ITime"))  # the first line that creates sec will create a local copy so we can use setattr() to avoid potential copy of class()<-
 }
@@ -238,16 +238,16 @@ rep.ITime = function(x, ...)
 round.ITime <- function(x, digits = c("hours", "minutes"), ...)
 {
   (setattr(switch(match.arg(digits),
-                  hours = as.integer(round(unclass(x)/3600)*3600),
-                  minutes = as.integer(round(unclass(x)/60)*60)),
+                  hours = as.integer(round(unclass(x)/3600.0)*3600.0),
+                  minutes = as.integer(round(unclass(x)/60.0)*60.0)),
            "class", "ITime"))
 }
 
 trunc.ITime <- function(x, units = c("hours", "minutes"), ...)
 {
   (setattr(switch(match.arg(units),
-                  hours = as.integer(unclass(x)%/%3600*3600),
-                  minutes = as.integer(unclass(x)%/%60*60)),
+                  hours = as.integer(unclass(x)%/%3600.0*3600.0),
+                  minutes = as.integer(unclass(x)%/%60.0*60.0)),
            "class", "ITime"))
 }
 
@@ -280,7 +280,7 @@ IDateTime.default = function(x, ...) {
 
 # POSIXt support
 
-as.POSIXct.IDate = function(x, tz = "UTC", time = 0, ...) {
+as.POSIXct.IDate = function(x, tz = "UTC", time = 0.0, ...) {
   if (missing(time) && inherits(tz, "ITime")) {
     time = tz # allows you to use time as the 2nd argument
     tz = "UTC"

R/between.R

@@ -76,8 +76,13 @@ inrange = function(x,lower,upper,incbounds=TRUE) {
   verbose = isTRUE(getOption("datatable.verbose"))
   if (verbose) {last.started.at=proc.time();catf("forderv(query) took ... ");flush.console()}
   if (verbose) {cat(timetaken(last.started.at),"\n"); flush.console()}
-  ans = bmerge(shallow(subject), query, 1L:2L, c(1L,1L),
-      0, c(FALSE, TRUE), 0L, "all", ops, verbose) # fix for #1819, turn on verbose messages
+  ans = bmerge(
+    shallow(subject), query,
+    icols=1L:2L, xcols=c(1L, 1L),
+    roll=0.0, rollends=c(FALSE, TRUE),
+    nomatch=0L, mult="all",
+    ops=ops, verbose=verbose # fix for #1819, turn on verbose messages
+  )
   xo = ans$xo
   options(datatable.verbose=FALSE)
   setDT(ans[c("starts", "lens")], key=c("starts", "lens"))

R/data.table.R

@@ -128,7 +128,7 @@ replace_dot_alias = function(e) {
   }
 }
 
-"[.data.table" = function(x, i, j, by, keyby, with=TRUE, nomatch=NA, mult="all", roll=FALSE, rollends=if (roll=="nearest") c(TRUE,TRUE) else if (roll>=0) c(FALSE,TRUE) else c(TRUE,FALSE), which=FALSE, .SDcols, verbose=getOption("datatable.verbose"), allow.cartesian=getOption("datatable.allow.cartesian"), drop=NULL, on=NULL, env=NULL, showProgress=getOption("datatable.showProgress", interactive()))
+"[.data.table" = function(x, i, j, by, keyby, with=TRUE, nomatch=NA, mult="all", roll=FALSE, rollends=if (roll=="nearest") c(TRUE,TRUE) else if (roll>=0.0) c(FALSE,TRUE) else c(TRUE,FALSE), which=FALSE, .SDcols, verbose=getOption("datatable.verbose"), allow.cartesian=getOption("datatable.allow.cartesian"), drop=NULL, on=NULL, env=NULL, showProgress=getOption("datatable.showProgress", interactive()))
 {
   # ..selfcount <<- ..selfcount+1  # in dev, we check no self calls, each of which doubles overhead, or could
   # test explicitly if the caller is [.data.table (even stronger test. TO DO.)
@@ -149,7 +149,7 @@ replace_dot_alias = function(e) {
   }
   if (!missing(verbose)) {
     if (!is.integer(verbose) && !is.logical(verbose)) stopf("verbose must be logical or integer")
-    if (length(verbose)!=1 || anyNA(verbose)) stopf("verbose must be length 1 non-NA")
+    if (length(verbose)!=1L || anyNA(verbose)) stopf("verbose must be length 1 non-NA")
     # set the global verbose option because that is fetched from C code without having to pass it through
     oldverbose = options(datatable.verbose=verbose)
     on.exit(options(oldverbose))
@@ -215,7 +215,7 @@ replace_dot_alias = function(e) {
   # TO DO (document/faq/example). Removed for now ... if ((roll || rolltolast) && missing(mult)) mult="last" # for when there is exact match to mult. This does not control cases where the roll is mult, that is always the last one.
   .unsafe.opt() #3585
   missingnomatch = missing(nomatch)
-  nomatch0 = identical(nomatch,0) || identical(nomatch,0L) || identical(nomatch, FALSE)  # for warning with row-numbers in i; #4353
+  nomatch0 = identical(nomatch,0.0) || identical(nomatch,0L) || identical(nomatch, FALSE)  # for warning with row-numbers in i; #4353
   if (nomatch0) nomatch=NULL  # retain nomatch=0|FALSE backwards compatibility, #857 #5214
   if (!is.null(nomatch)) {
     if (!(length(nomatch)==1L && is.na(nomatch))) stopf("nomatch= must be either NA or NULL (or 0 for backwards compatibility which is the same as NULL but please use NULL)")
@@ -572,7 +572,7 @@ replace_dot_alias = function(e) {
           ## benchmarks have shown that starting with 1e6 irows, a tweak can significantly reduce time
           ## (see #2366)
           if (verbose) {last.started.at=proc.time();catf("Reordering %d rows after bmerge done in ... ", length(irows));flush.console()}
-          if(length(irows) < 1e6){
+          if(length(irows) < 1e6L){
             irows = fsort(irows, internal=TRUE) ## internally, fsort on integer falls back to forderv
             } else {
               irows = as.integer(fsort(as.numeric(irows))) ## nocov; parallelized for numeric, but overhead of type conversion
@@ -1901,20 +1901,20 @@ replace_dot_alias = function(e) {
     }
 
     # adding ghead/gtail(n) support for n > 1 #5060 #523
-    q3 = 0
+    q3 = 0L
     if (!is.symbol(jsub)) {
       headTail_arg = function(q) {
         if (length(q)==3L && length(q3 <- q[[3L]])==1L && is.numeric(q3) &&
-         (q[[1L]]) %chin% c("ghead", "gtail") && q3!=1) q3
-        else 0
+         (q[[1L]]) %chin% c("ghead", "gtail") && q3!=1L) q3
+        else 0L
       }
       if (jsub %iscall% "list"){
         q3 = max(sapply(jsub, headTail_arg))
       } else if (length(jsub)==3L) {
         q3 = headTail_arg(jsub)
       }
     }
-    if (q3 > 0) {
+    if (q3 > 0L) {
       grplens = pmin.int(q3, len__)
       g = lapply(g, rep.int, times=grplens)
     } else if (.is_nrows(jsub)) {
@@ -2738,7 +2738,7 @@ setcolorder = function(x, neworder=key(x), before=NULL, after=NULL, skip_absent=
   if (length(before)>1L || length(after)>1L)
     stopf("before=/after= accept a single column name or number, not more than one")
   neworder = colnamesInt(x, neworder, check_dups=FALSE, skip_absent=skip_absent)  # dups are now checked inside Csetcolorder below
-  neworder = neworder[neworder != 0] # tests 498.11, 498.13 fail w/o this
+  neworder = neworder[neworder != 0L] # tests 498.11, 498.13 fail w/o this
   if (length(before))
     neworder = c(setdiff(seq_len(colnamesInt(x, before) - 1L), neworder), neworder)
   if (length(after))
@@ -3089,10 +3089,10 @@ gweighted.mean = function(x, w, ..., na.rm=FALSE) {
   else {
     if (na.rm) { # take those indices out of the equation by setting them to 0
       ix <- is.na(x)
-      x[ix] <- 0
-      w[ix] <- 0
+      x[ix] <- 0.0
+      w[ix] <- 0.0
     }
-    gsum((w!=0)*x*w, na.rm=FALSE)/gsum(w, na.rm=FALSE)
+    gsum((w!=0.0)*x*w, na.rm=FALSE)/gsum(w, na.rm=FALSE)
   }
 }
 gprod = function(x, na.rm=FALSE) .Call(Cgprod, x, na.rm)
@@ -3246,7 +3246,7 @@ is_constantish = function(q, check_singleton=FALSE) {
       ## redirect to normal DT[x == TRUE]
       stub = call("==", as.symbol(col), TRUE)
     }
-    if (length(stub[[1L]]) != 1) return(NULL) # nocov Whatever it is, definitely not one of the valid operators
+    if (length(stub[[1L]]) != 1L) return(NULL) # nocov Whatever it is, definitely not one of the valid operators
     operator = as.character(stub[[1L]])
     if (!operator %chin% validOps$op) return(NULL) ## operator not supported
     if (!is.name(stub[[2L]])) return(NULL)
@@ -3287,7 +3287,7 @@ is_constantish = function(q, check_singleton=FALSE) {
   }
   if (length(i) == 0L) internal_error("i became length-0") # nocov
   ## convert i to data.table with all combinations in rows.
-  if(length(i) > 1L && prod(lengths(i)) > 1e4){
+  if(length(i) > 1L && prod(lengths(i)) > 1e4L){
     ## CJ would result in more than 1e4 rows. This would be inefficient, especially memory-wise #2635
     if (verbose) {catf("Subsetting optimization disabled because the cross-product of RHS values exceeds 1e4, causing memory problems.\n");flush.console()}
     return(NULL)

R/fdroplevels.R

@@ -19,7 +19,7 @@ setdroplevels = function(x, except=NULL, exclude=NULL) {
   if (!nrow(x)) return(invisible(x))
   ix = vapply_1b(x, is.factor)
   if (!is.null(except)) {
-    stopifnot(is.numeric(except), except >= 1, except <= length(x))
+    stopifnot(is.numeric(except), except >= 1L, except <= length(x))
     ix[except] = FALSE
   }
   if (!any(ix)) return(invisible(x))

R/fmelt.R

@@ -31,7 +31,7 @@ patterns = function(..., cols=character(0L), ignore.case=FALSE, perl=FALSE, fixe
 
 measure = function(..., sep="_", pattern, cols, multiple.keyword="value.name") {
   mcall = match.call()
-  L = as.list(mcall)[-1]
+  L = as.list(mcall)[-1L]
   formal.names = names(formals())
   formal.i.vec = which(names(L) %in% formal.names)
   fun.list = L[-formal.i.vec]
@@ -52,7 +52,7 @@ measure = function(..., sep="_", pattern, cols, multiple.keyword="value.name") {
   # evaluate each value in ... and stop if not function.
   for (fun.i in which(user.named)) {
     fun = eval(fun.list[[fun.i]], parent.frame(1L))
-    if (!is.function(fun) || length(formals(args(fun)))==0) {
+    if (!is.function(fun) || length(formals(args(fun)))==0L) {
       stopf("each ... argument to measure must be a function with at least one argument, problem: %s", names(fun.list)[[fun.i]])
     }
     fun.list[[fun.i]] = fun
@@ -66,7 +66,7 @@ measurev = function(fun.list, sep="_", pattern, cols, multiple.keyword="value.na
   if (!missing(sep) && !missing(pattern)) {
     stopf("both sep and pattern arguments used; must use either sep or pattern (not both)")
   }
-  if (!(is.character(multiple.keyword) && length(multiple.keyword)==1 && !is.na(multiple.keyword) && nzchar(multiple.keyword))) {
+  if (!(is.character(multiple.keyword) && length(multiple.keyword)==1L && !is.na(multiple.keyword) && nzchar(multiple.keyword))) {
     stopf("multiple.keyword must be a character string with nchar>0")
   }
   if (!is.character(cols)) {
@@ -89,7 +89,7 @@ measurev = function(fun.list, sep="_", pattern, cols, multiple.keyword="value.na
       stopf("pattern must be character string")
     }
     match.vec = regexpr(pattern, cols, perl=TRUE)
-    measure.vec.i = which(0 < match.vec)
+    measure.vec.i = which(match.vec > 0L)
     if (length(measure.vec.i) == 0L) {
       stopf("pattern did not match any cols, so nothing would be melted; fix by changing pattern")
     }
@@ -111,7 +111,7 @@ measurev = function(fun.list, sep="_", pattern, cols, multiple.keyword="value.na
     list.of.vectors = strsplit(cols, sep, fixed=TRUE)
     vector.lengths = lengths(list.of.vectors)
     n.groups = max(vector.lengths)
-    if (n.groups == 1) {
+    if (n.groups == 1L) {
       stopf("each column name results in only one item after splitting using sep, which means that all columns would be melted; to fix please either specify melt on all columns directly without using measure, or use a different sep/pattern specification")
     }
     if (n.groups != length(fun.list)) {
@@ -135,7 +135,7 @@ measurev = function(fun.list, sep="_", pattern, cols, multiple.keyword="value.na
   for (group.i in fun.i.vec) {
     group.name = names(fun.list)[[group.i]]
     fun = fun.list[[group.i]]
-    if (!is.function(fun) || length(formals(args(fun)))==0) {
+    if (!is.function(fun) || length(formals(args(fun))) == 0L) {
       stopf("in the measurev fun.list, each non-NULL element must be a function with at least one argument, problem: %s", group.name)
     }
     group.val = fun(group.dt[[group.name]])

R/foverlaps.R

@@ -90,7 +90,7 @@ foverlaps = function(x, y, by.x=if (!is.null(key(x))) key(x) else key(y), by.y=k
     # next representative double > x under the given precision (48,56 or 64-bit in data.table) = x*incr
     dt_eps = function() {
       bits = floor(log2(.Machine$double.eps))
-      2 ^ (bits + (getNumericRounding() * 8L))
+      2L ^ (bits + (getNumericRounding() * 8L))
     }
     isdouble = TRUE
     isposix = "POSIXct" %chin% yclass