nafill logical vector support (#7537)

* nafill logical vector support

* documentation

* error message

Author: MichaelChirico

NEWS.md

@@ -12,6 +12,10 @@
 
 3. options `"datatable.old.matrix.autoname"` is now `FALSE` by default, meaning `names(data.table(x=1, cbind(1)))` is now `c("x", "V2")`. Toggle the option to retain the old behavior for now; future releases will work to remove this possibility. See the release notes for 1.18.0, item 1 under `NOTE OF INTENDED FUTURE POTENTIAL BREAKING CHANGES`.
 
+### NEW FEATURES
+
+1. `nafill()`, `setnafill()` extended to work on logical vectors (part of [#3992](https://github.com/Rdatatable/data.table/issues/3992)). Thanks @jangorecki for the request and @MichaelChirico for the PR.
+
 ### Notes
 
 1. {data.table} now depends on R 3.5.0 (2018).

inst/tests/nafill.Rraw

@@ -31,7 +31,7 @@ test(1.06, nafill(x, fill=NA), x)
 test(1.07, nafill(x, fill=NA_real_), x)
 test(1.08, nafill(x, fill=Inf), x, warning="precision lost")
 test(1.09, nafill(x, fill=NaN), x)
-y = x/2
+y = x/2 # double input
 test(1.11, nafill(y, "locf"), c(NA,NA,3,4,4,4,7,8,8,8)/2)
 test(1.12, nafill(y, "nocb"), c(3,3,3,4,7,7,7,8,NA,NA)/2)
 test(1.13, nafill(y, fill=0L), c(0,0,3,4,0,0,7,8,0,0)/2)
@@ -112,8 +112,8 @@ x = 1:10
 test(3.01, nafill(x, "locf", fill=0L), x)
 test(3.02, setnafill(list(copy(x)), "locf", fill=0L), list(x))
 test(3.03, setnafill(x, "locf"), error="in-place update is supported only for list")
-test(3.04, nafill(letters[1:5], fill=0), error="must be numeric type, or list/data.table")
-test(3.05, setnafill(list(letters[1:5]), fill=0), error="must be numeric type, or list/data.table")
+test(3.04, nafill(letters[1:5], fill=0), error="must be logical/numeric type, or list/data.table")
+test(3.05, setnafill(list(letters[1:5]), fill=0), error="must be logical/numeric type, or list/data.table")
 test(3.06, nafill(x, fill=1:2), error="fill must be a vector of length 1.*fcoalesce")
 test(3.07, nafill(x, "locf", fill=1:2), error="fill must be a vector of length 1.*x\\.$")
 test(3.08, nafill(x, fill="asd"), x, warning=c("Coercing.*character.*integer","NAs introduced by coercion"))
@@ -320,6 +320,19 @@ test(11.08, coerceAs(a, 1L), error="must not be matrix or array")
 test(11.09, coerceAs(1L, a), error="must not be matrix or array")
 
 # nafill, setnafill for character, factor and other types #3992
+## logical input
+x = c(NA, NA, TRUE, FALSE, NA, NA, FALSE, TRUE, NA, NA)
+test(12.01, nafill(x, "locf"), c(NA, NA, TRUE, FALSE, FALSE, FALSE, FALSE, TRUE, TRUE, TRUE))
+test(12.02, nafill(x, "nocb"), c(TRUE, TRUE, TRUE, FALSE, FALSE, FALSE, FALSE, TRUE, NA, NA))
+test(12.03, nafill(x, fill=TRUE), c(TRUE, TRUE, TRUE, FALSE, TRUE, TRUE, FALSE, TRUE, TRUE, TRUE))
+test(12.04, nafill(x, fill=0L), c(FALSE, FALSE, TRUE, FALSE, FALSE, FALSE, FALSE, TRUE, FALSE, FALSE))
+test(12.05, nafill(x, fill=5.0), c(TRUE, TRUE, TRUE, FALSE, TRUE, TRUE, FALSE, TRUE, TRUE, TRUE), warning="double.*taken as TRUE")
+test(12.06, nafill(x, fill=Inf), c(TRUE, TRUE, TRUE, FALSE, TRUE, TRUE, FALSE, TRUE, TRUE, TRUE), warning="double.*taken as TRUE")
+test(12.07, nafill(x, fill=NA), x)
+test(12.08, nafill(x, fill=NA_integer_), x)
+test(12.09, nafill(x, fill=NA_real_), x)
+test(12.10, nafill(x, fill=NaN), x)
+
 ## logical
 ## character
 ## factor

man/nafill.Rd

@@ -14,9 +14,9 @@ nafill(x, type=c("const", "locf", "nocb"), fill=NA, nan=NA)
 setnafill(x, type=c("const", "locf", "nocb"), fill=NA, nan=NA, cols=seq_along(x))
 }
 \arguments{
-  \item{x}{ Vector, list, data.frame or data.table of numeric columns. }
+  \item{x}{ Vector, list, data.frame or data.table of logical/numeric columns. }
   \item{type}{ Character, one of \emph{"const"}, \emph{"locf"} or \emph{"nocb"}. Defaults to \code{"const"}. }
-  \item{fill}{ Numeric value to be used to replace missing observations. See examples. }
+  \item{fill}{ Value to be used to replace missing observations. See examples. }
   \item{nan}{ Either \code{NaN} or \code{NA}; if the former, \code{NaN} is treated as distinct from \code{NA}, otherwise, they are treated the same during replacement. See Examples. }
   \item{cols}{ Numeric or character vector specifying columns to be updated. }
 }

src/nafill.c

@@ -113,8 +113,8 @@ SEXP nafillR(SEXP obj, SEXP type, SEXP fill, SEXP nan_is_na_arg, SEXP inplace, S
   if (obj_scalar) {
     if (binplace)
       error(_("'x' argument is atomic vector, in-place update is supported only for list/data.table"));
-    else if (!isReal(obj) && !isInteger(obj))
-      error(_("'x' argument must be numeric type, or list/data.table of numeric types"));
+    else if (!isReal(obj) && !isInteger(obj) && !isLogical(obj))
+      error(_("'x' argument must be logical/numeric type, or list/data.table of logical/numeric types"));
     SEXP obj1 = obj;
     obj = PROTECT(allocVector(VECSXP, 1)); protecti++; // wrap into list
     SET_VECTOR_ELT(obj, 0, obj1);
@@ -124,8 +124,8 @@ SEXP nafillR(SEXP obj, SEXP type, SEXP fill, SEXP nan_is_na_arg, SEXP inplace, S
   int *icols = INTEGER(ricols);
   for (int i=0; i<length(ricols); i++) {
     SEXP this_col = VECTOR_ELT(obj, icols[i]-1);
-    if (!isReal(this_col) && !isInteger(this_col))
-      error(_("'x' argument must be numeric type, or list/data.table of numeric types"));
+    if (!isReal(this_col) && !isInteger(this_col) && !isLogical(this_col))
+      error(_("'x' argument must be logical/numeric type, or list/data.table of logical/numeric types"));
     SET_VECTOR_ELT(x, i, this_col);
   }
   R_len_t nx = length(x);
@@ -210,7 +210,7 @@ SEXP nafillR(SEXP obj, SEXP type, SEXP fill, SEXP nan_is_na_arg, SEXP inplace, S
         nafillDouble(dx[i], inx[i], itype, hasFill ? ((double *)fillp[i])[0] : NA_REAL, nan_is_na, &vans[i], verbose);
       }
     } break;
-    case INTSXP : {
+    case LGLSXP: case INTSXP : {
       nafillInteger(ix[i], inx[i], itype, hasFill ? ((int32_t *)fillp[i])[0] : NA_INTEGER, &vans[i], verbose);
     } break;
     }