frollapply correctly catches negative n (#7293)

* all

* extend news

* mark experimental

* codecov

* also test for negative n

* test number

* trailing ws

* unit tests

* unit tests, after save file

* Apply suggestions from code review

Co-authored-by: Michael Chirico <[email protected]>

* frolladapt nicer news example

* fix missed conflict in merge

---------

Co-authored-by: Michael Chirico <[email protected]>

Author: jangorecki

R/frollapply.R

@@ -61,6 +61,7 @@ all_data.frame = function(x) all(vapply_1b(x, is.data.frame, use.names=FALSE))
 all_list = function(x) all(vapply_1b(x, is.list, use.names=FALSE))
 equal.lengths = function(x) length(unique(lengths(x))) <= 1L
 equal.nrows = function(x) length(unique(vapply(x, nrow, 0L))) <= 1L
+anyNAneg = function(x) anyNA(x) || any(x < 0L)
 
 frollapply = function(X, N, FUN, ..., by.column=TRUE, fill=NA, align=c("right","left","center"), adaptive=FALSE, partial=FALSE, give.names=FALSE, simplify=TRUE, x, n) {
   if (!missing(x)) {
@@ -147,7 +148,7 @@ frollapply = function(X, N, FUN, ..., by.column=TRUE, fill=NA, align=c("right","
     nnam = names(N) ## used for give.names
     if (!is.integer(N))
       N = as.integer(N)
-    if (anyNA(N))
+    if (anyNAneg(N))
       stopf("'N' must be non-negative integer values (>= 0)")
     nn = length(N) ## top level loop for vectorized n
   } else {
@@ -158,7 +159,7 @@ frollapply = function(X, N, FUN, ..., by.column=TRUE, fill=NA, align=c("right","
         stopf("length of integer vector(s) provided as list to 'N' argument must be equal to number of observations provided in 'X'")
       if (!is.integer(N))
         N = as.integer(N)
-      if (anyNA(N))
+      if (anyNAneg(N))
         stopf("'N' must be non-negative integer values (>= 0)")
       nn = 1L
       N = list(N)
@@ -172,7 +173,7 @@ frollapply = function(X, N, FUN, ..., by.column=TRUE, fill=NA, align=c("right","
         stopf("'N' must be an integer vector or list of integer vectors")
       if (!all(vapply_1b(N, is.integer, use.names=FALSE)))
         N = lapply(N, as.integer)
-      if (any(vapply_1b(N, anyNA, use.names=FALSE)))
+      if (any(vapply_1b(N, anyNAneg, use.names=FALSE)))
         stopf("'N' must be non-negative integer values (>= 0)")
       nn = length(N)
       nnam = names(N)

inst/tests/froll.Rraw

@@ -1005,7 +1005,7 @@ test(6000.502, frollmax(c(5,NaN,1), 1L), c(5,NaN,1))
 test(6000.503, frollmax(c(5,1,1,NaN,1,1,1), 2L), c(NA,5,1,NaN,NaN,1,1))
 test(6000.504, frollmax(c(5,1,NA,NaN,1,1,1), 2L), c(NA,5,NA,NA,NaN,1,1))
 
-# n==NA
+# n==NA, n<0
 test(6000.550, frollmean(1:3, NA), error="'n' must be an integer")
 test(6000.551, frollmean(1:3, NA_integer_), error="'n' must be non-negative integer values (>= 0)")
 test(6000.552, frollmean(1:3, NA, algo="exact"), error="'n' must be an integer")
@@ -1016,6 +1016,11 @@ test(6000.556, frollapply(FUN=mean, 1:3, NA), error="'N' must be an integer")
 test(6000.557, frollapply(FUN=mean, 1:3, NA_integer_), error="'N' must be non-negative integer values (>= 0)")
 test(6000.558, frollapply(FUN=mean, adaptive=TRUE, 1:3, c(2,NA,2)), error="'N' must be non-negative integer values (>= 0)")
 test(6000.559, frollapply(FUN=mean, adaptive=TRUE, 1:3, list(c(2,NA,2))), error="'N' must be non-negative integer values (>= 0)")
+test(6000.560, frollmean(1:3, -1), error="'n' must be non-negative integer values (>= 0)")
+test(6000.561, frollmean(1:3, -1, algo="exact"), error="'n' must be non-negative integer values (>= 0)")
+test(6000.562, frollapply(FUN=mean, 1:3, -1), error="'N' must be non-negative integer values (>= 0)")
+test(6000.563, frollapply(FUN=mean, 1:3, c(0,-1,1), adaptive=TRUE), error="'N' must be non-negative integer values (>= 0)")
+test(6000.564, frollapply(FUN=mean, 1:3, list(c(0,-1,1)), adaptive=TRUE), error="'N' must be non-negative integer values (>= 0)")
 
 # n==0, k==0, k[i]==0
 test(6001.111, frollmean(1:3, 0), c(NaN,NaN,NaN), options=c("datatable.verbose"=TRUE), output="window width of size 0")