From dd875ac76df9d0fdde19b632ebdce26be41066f9 Mon Sep 17 00:00:00 2001 From: venom1204 Date: Sat, 18 Jul 2026 10:10:27 +0000 Subject: [PATCH 01/21] updated changes --- R/shift.R | 12 ++++-- inst/tests/tests.Rraw | 6 +++ man/nafill.Rd | 11 +++++- src/data.table.h | 8 ++-- src/nafill.c | 88 +++++++++++++++++++++++++++++++++---------- 5 files changed, 97 insertions(+), 28 deletions(-) diff --git a/R/shift.R b/R/shift.R index 1c68d13c41..c586bed0b6 100644 --- a/R/shift.R +++ b/R/shift.R @@ -26,12 +26,16 @@ shift = function(x, n=1L, fill, type=c("lag", "lead", "shift", "cyclic"), give.n ans } -nafill = function(x, type=c("const","locf","nocb"), fill=NA, nan=NA) { +nafill = function(x, type=c("const","locf","nocb"), fill=NA, nan=NA, limit=Inf) { type = match.arg(type) - .Call(CnafillR, x, type, fill, nan_is_na(nan), FALSE, NULL) + if (!is.numeric(limit) || length(limit) != 1L || limit < 0) + stopf("limit must be a non-negative scalar numeric or Inf") + .Call(CnafillR, x, type, fill, nan_is_na(nan), FALSE, NULL, as.double(limit)) } -setnafill = function(x, type=c("const","locf","nocb"), fill=NA, nan=NA, cols=seq_along(x)) { +setnafill = function(x, type=c("const","locf","nocb"), fill=NA, nan=NA, cols=seq_along(x), limit=Inf) { type = match.arg(type) - invisible(.Call(CnafillR, x, type, fill, nan_is_na(nan), TRUE, cols)) + if (!is.numeric(limit) || length(limit) != 1L || limit < 0) + stopf("limit must be a non-negative scalar numeric or Inf") + invisible(.Call(CnafillR, x, type, fill, nan_is_na(nan), TRUE, cols, as.double(limit))) } diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 00973ae3a5..5c72eca1de 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21831,3 +21831,9 @@ test(2378.92, .Call(CresizeVector, x, 2:3), error = "must be length 1 non-NA non test(2378.93, .Call(CresizeVector, x, -2L), error = "must be length 1 non-NA non-negative integer value") test(2378.94, .Call(CresizeVector, x, NA_integer_), error = "must be length 1 non-NA non-negative integer value") test(2378.95, .Call(CresizeVector, NULL, 2L), error = "must be a vector") + +# #7677: add limit argument to nafill() +test(2379.01, nafill(c(1, NA, NA, NA, 5), type="locf", limit=1), c(1, 1, NA, NA, 5)) +test(2379.02, nafill(c(1, NA, NA, 5, NA, NA, NA, 9), type="locf", limit=2), c(1, 1, 1, 5, 5, 5, NA, 9)) +test(2379.03, nafill(c(1, NA, NA, 4, NA), type="locf", limit=Inf), c(1, 1, 1, 4, 4)) +test(2379.04, nafill(c(NA, NA, 3, NA, NA), type="locf", limit=1), c(NA, NA, 3, 3, NA)) diff --git a/man/nafill.Rd b/man/nafill.Rd index 90c4b1c5c6..57c2742b8e 100644 --- a/man/nafill.Rd +++ b/man/nafill.Rd @@ -10,8 +10,8 @@ Fast fill missing values using constant value, \emph{last observation carried forward} or \emph{next observation carried backward}. } \usage{ -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)) +nafill(x, type=c("const", "locf", "nocb"), fill=NA, nan=NA, limit=Inf) +setnafill(x, type=c("const", "locf", "nocb"), fill=NA, nan=NA, cols=seq_along(x), limit=Inf) } \arguments{ \item{x}{ Vector, list, data.frame or data.table of logical, numeric or character columns. } @@ -19,6 +19,7 @@ setnafill(x, type=c("const", "locf", "nocb"), fill=NA, nan=NA, cols=seq_along(x) \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. } + \item{limit}{ The maximum number of consecutive \code{NA} values to fill. Must be a non-negative scalar numeric. Default is \code{Inf}. } } \details{ Supported types are \emph{logical}, \emph{integer}, \emph{double}, \emph{character}, and \emph{factor}, as well as classes built on top of these such as \code{Date}, \code{IDate}, and \code{POSIXct}. @@ -51,6 +52,12 @@ nafill(dt, "nocb") setnafill(dt, "locf", cols=c("v2","v3")) dt + +# limit= restricts the number of consecutive fills +y = c(1, NA, NA, NA, 5) +nafill(y, "locf", limit=1) # Only fills the first NA +nafill(y, "locf", limit=2) # Fills the first two NAs + } \seealso{ \code{\link{shift}}, \code{\link{data.table}}, \code{\link{fcoalesce}} diff --git a/src/data.table.h b/src/data.table.h index dbfe495edb..9427ff7745 100644 --- a/src/data.table.h +++ b/src/data.table.h @@ -311,9 +311,11 @@ SEXP copyAsGrowable(SEXP x); SEXP resizeVector(SEXP x, SEXP size); // nafill.c -void nafillDouble(double *x, uint_fast64_t nx, unsigned int type, double fill, bool nan_is_na, ans_t *ans, bool verbose); -void nafillInteger(int32_t *x, uint_fast64_t nx, unsigned int type, int32_t fill, ans_t *ans, bool verbose); -SEXP nafillR(SEXP obj, SEXP type, SEXP fill, SEXP nan_is_na_arg, SEXP inplace, SEXP cols); +void nafillDouble(double *x, uint_fast64_t nx, unsigned int type, double fill, bool nan_is_na, ans_t *ans, bool verbose, double limit); +void nafillInteger(int32_t *x, uint_fast64_t nx, unsigned int type, int32_t fill, ans_t *ans, bool verbose, double limit); +void nafillInteger64(int64_t *x, uint_fast64_t nx, unsigned int type, int64_t fill, ans_t *ans, bool verbose, double limit); +void nafillString(const SEXP *x, uint_fast64_t nx, unsigned int type, SEXP fill, ans_t *ans, bool verbose, double limit); +SEXP nafillR(SEXP obj, SEXP type, SEXP fill, SEXP nan_is_na_arg, SEXP inplace, SEXP cols, SEXP limit); // between.c SEXP between(SEXP x, SEXP lower, SEXP upper, SEXP incbounds, SEXP NAbounds, SEXP check); diff --git a/src/nafill.c b/src/nafill.c index 8a8e8eab56..197fe954cb 100644 --- a/src/nafill.c +++ b/src/nafill.c @@ -1,6 +1,6 @@ #include "data.table.h" -void nafillDouble(double *x, uint_fast64_t nx, unsigned int type, double fill, bool nan_is_na, ans_t *ans, bool verbose) { +void nafillDouble(double *x, uint_fast64_t nx, unsigned int type, double fill, bool nan_is_na, ans_t *ans, bool verbose, double limit) { double tic=0.0; if (verbose) tic = omp_get_wtime(); @@ -15,34 +15,52 @@ void nafillDouble(double *x, uint_fast64_t nx, unsigned int type, double fill, b } } } else if (type==1) { // locf + uint_fast64_t fills = 0; if (nan_is_na) { ans->dbl_v[0] = ISNAN(x[0]) ? fill : x[0]; + if (ISNAN(x[0])) fills = 1; for (uint_fast64_t i=1; idbl_v[i] = ISNAN(x[i]) ? ans->dbl_v[i-1] : x[i]; + if (ISNAN(x[i])) { + if (fills < limit) { ans->dbl_v[i] = ans->dbl_v[i-1]; fills++; } + else ans->dbl_v[i] = x[i]; + } else { ans->dbl_v[i] = x[i]; fills = 0; } } } else { ans->dbl_v[0] = ISNA(x[0]) ? fill : x[0]; + if (ISNA(x[0])) fills = 1; for (uint_fast64_t i=1; idbl_v[i] = ISNA(x[i]) ? ans->dbl_v[i-1] : x[i]; + if (ISNA(x[i])) { + if (fills < limit) { ans->dbl_v[i] = ans->dbl_v[i-1]; fills++; } + else ans->dbl_v[i] = x[i]; + } else { ans->dbl_v[i] = x[i]; fills = 0; } } } } else if (type==2) { // nocb + uint_fast64_t fills = 0; if (nan_is_na) { ans->dbl_v[nx-1] = ISNAN(x[nx-1]) ? fill : x[nx-1]; + if (ISNAN(x[nx-1])) fills = 1; for (int_fast64_t i=nx-2; i>=0; i--) { - ans->dbl_v[i] = ISNAN(x[i]) ? ans->dbl_v[i+1] : x[i]; + if (ISNAN(x[i])) { + if (fills < limit) { ans->dbl_v[i] = ans->dbl_v[i+1]; fills++; } + else ans->dbl_v[i] = x[i]; + } else { ans->dbl_v[i] = x[i]; fills = 0; } } } else { ans->dbl_v[nx-1] = ISNA(x[nx-1]) ? fill : x[nx-1]; + if (ISNA(x[nx-1])) fills = 1; for (int_fast64_t i=nx-2; i>=0; i--) { - ans->dbl_v[i] = ISNA(x[i]) ? ans->dbl_v[i+1] : x[i]; + if (ISNA(x[i])) { + if (fills < limit) { ans->dbl_v[i] = ans->dbl_v[i+1]; fills++; } + else ans->dbl_v[i] = x[i]; + } else { ans->dbl_v[i] = x[i]; fills = 0; } } } } if (verbose) snprintf(ans->message[0], 500, _("%s: took %.3fs\n"), __func__, omp_get_wtime()-tic); } -void nafillInteger(int32_t *x, uint_fast64_t nx, unsigned int type, int32_t fill, ans_t *ans, bool verbose) { +void nafillInteger(int32_t *x, uint_fast64_t nx, unsigned int type, int32_t fill, ans_t *ans, bool verbose, double limit) { double tic=0.0; if (verbose) tic = omp_get_wtime(); @@ -51,20 +69,30 @@ void nafillInteger(int32_t *x, uint_fast64_t nx, unsigned int type, int32_t fill ans->int_v[i] = x[i]==NA_INTEGER ? fill : x[i]; } } else if (type==1) { // locf + uint_fast64_t fills = 0; ans->int_v[0] = x[0]==NA_INTEGER ? fill : x[0]; + if (x[0]==NA_INTEGER) fills = 1; for (uint_fast64_t i=1; iint_v[i] = x[i]==NA_INTEGER ? ans->int_v[i-1] : x[i]; + if (x[i]==NA_INTEGER) { + if (fills < limit) { ans->int_v[i] = ans->int_v[i-1]; fills++; } + else ans->int_v[i] = x[i]; + } else { ans->int_v[i] = x[i]; fills = 0; } } } else if (type==2) { // nocb + uint_fast64_t fills = 0; ans->int_v[nx-1] = x[nx-1]==NA_INTEGER ? fill : x[nx-1]; + if (x[nx-1]==NA_INTEGER) fills = 1; for (int_fast64_t i=nx-2; i>=0; i--) { - ans->int_v[i] = x[i]==NA_INTEGER ? ans->int_v[i+1] : x[i]; + if (x[i]==NA_INTEGER) { + if (fills < limit) { ans->int_v[i] = ans->int_v[i+1]; fills++; } + else ans->int_v[i] = x[i]; + } else { ans->int_v[i] = x[i]; fills = 0; } } } if (verbose) snprintf(ans->message[0], 500, _("%s: took %.3fs\n"), __func__, omp_get_wtime()-tic); } -void nafillInteger64(int64_t *x, uint_fast64_t nx, unsigned int type, int64_t fill, ans_t *ans, bool verbose) { +void nafillInteger64(int64_t *x, uint_fast64_t nx, unsigned int type, int64_t fill, ans_t *ans, bool verbose, double limit) { double tic=0.0; if (verbose) tic = omp_get_wtime(); @@ -73,21 +101,31 @@ void nafillInteger64(int64_t *x, uint_fast64_t nx, unsigned int type, int64_t fi ans->int64_v[i] = x[i]==NA_INTEGER64 ? fill : x[i]; } } else if (type==1) { // locf + uint_fast64_t fills = 0; ans->int64_v[0] = x[0]==NA_INTEGER64 ? fill : x[0]; + if (x[0]==NA_INTEGER64) fills = 1; for (uint_fast64_t i=1; iint64_v[i] = x[i]==NA_INTEGER64 ? ans->int64_v[i-1] : x[i]; + if (x[i]==NA_INTEGER64) { + if (fills < limit) { ans->int64_v[i] = ans->int64_v[i-1]; fills++; } + else ans->int64_v[i] = x[i]; + } else { ans->int64_v[i] = x[i]; fills = 0; } } } else if (type==2) { // nocb + uint_fast64_t fills = 0; ans->int64_v[nx-1] = x[nx-1]==NA_INTEGER64 ? fill : x[nx-1]; + if (x[nx-1]==NA_INTEGER64) fills = 1; for (int_fast64_t i=nx-2; i>=0; i--) { - ans->int64_v[i] = x[i]==NA_INTEGER64 ? ans->int64_v[i+1] : x[i]; + if (x[i]==NA_INTEGER64) { + if (fills < limit) { ans->int64_v[i] = ans->int64_v[i+1]; fills++; } + else ans->int64_v[i] = x[i]; + } else { ans->int64_v[i] = x[i]; fills = 0; } } } if (verbose) snprintf(ans->message[0], 500, _("%s: took %.3fs\n"), __func__, omp_get_wtime()-tic); } -void nafillString(const SEXP *x, uint_fast64_t nx, unsigned int type, SEXP fill, ans_t *ans, bool verbose) { +void nafillString(const SEXP *x, uint_fast64_t nx, unsigned int type, SEXP fill, ans_t *ans, bool verbose, double limit) { double tic=0.0; if (verbose) tic = omp_get_wtime(); @@ -96,16 +134,26 @@ void nafillString(const SEXP *x, uint_fast64_t nx, unsigned int type, SEXP fill, SET_STRING_ELT(ans->char_v, i, x[i]==NA_STRING ? fill : x[i]); } } else if (type==1) { // locf + uint_fast64_t fills = 0; SET_STRING_ELT(ans->char_v, 0, x[0]==NA_STRING ? fill : x[0]); + if (x[0]==NA_STRING) fills = 1; const SEXP* thisans = SEXPPTR_RO(ans->char_v); // takes out STRING_ELT from loop for (uint_fast64_t i=1; ichar_v, i, x[i]==NA_STRING ? thisans[i-1] : x[i]); + if (x[i]==NA_STRING) { + if (fills < limit) { SET_STRING_ELT(ans->char_v, i, thisans[i-1]); fills++; } + else SET_STRING_ELT(ans->char_v, i, x[i]); + } else { SET_STRING_ELT(ans->char_v, i, x[i]); fills = 0; } } } else if (type==2) { // nocb + uint_fast64_t fills = 0; SET_STRING_ELT(ans->char_v, nx-1, x[nx-1]==NA_STRING ? fill : x[nx-1]); + if (x[nx-1]==NA_STRING) fills = 1; const SEXP* thisans = SEXPPTR_RO(ans->char_v); // takes out STRING_ELT from loop for (int_fast64_t i=nx-2; i>=0; i--) { - SET_STRING_ELT(ans->char_v, i, x[i]==NA_STRING ? thisans[i+1] : x[i]); + if (x[i]==NA_STRING) { + if (fills < limit) { SET_STRING_ELT(ans->char_v, i, thisans[i+1]); fills++; } + else SET_STRING_ELT(ans->char_v, i, x[i]); + } else { SET_STRING_ELT(ans->char_v, i, x[i]); fills = 0; } } } if (verbose) @@ -117,7 +165,7 @@ void nafillString(const SEXP *x, uint_fast64_t nx, unsigned int type, SEXP fill, over columns of the input data. This includes handling different data types and applying the designated filling method to each column in parallel. */ -SEXP nafillR(SEXP obj, SEXP type, SEXP fill, SEXP nan_is_na_arg, SEXP inplace, SEXP cols) { +SEXP nafillR(SEXP obj, SEXP type, SEXP fill, SEXP nan_is_na_arg, SEXP inplace, SEXP cols, SEXP limit) { int protecti=0; const bool verbose = GetVerbose(); @@ -128,6 +176,8 @@ SEXP nafillR(SEXP obj, SEXP type, SEXP fill, SEXP nan_is_na_arg, SEXP inplace, S if (verbose) tic = omp_get_wtime(); + double limit_val = REAL(limit)[0]; + bool copy = !LOGICAL(inplace)[0]; if (!IS_TRUE_OR_FALSE(nan_is_na_arg)) error(_("'%s' must be TRUE or FALSE"), "nan_is_na"); // # nocov @@ -253,16 +303,16 @@ SEXP nafillR(SEXP obj, SEXP type, SEXP fill, SEXP nan_is_na_arg, SEXP inplace, S switch (TYPEOF(VECTOR_ELT(x, i))) { case REALSXP : { if (isInt64[i]) { - nafillInteger64(i64x[i], inx[i], itype, hasFill ? ((int64_t *)fillp[i])[0] : NA_INTEGER64, &vans[i], verbose); + nafillInteger64(i64x[i], inx[i], itype, hasFill ? ((int64_t *)fillp[i])[0] : NA_INTEGER64, &vans[i], verbose, limit_val); } else { - nafillDouble(dx[i], inx[i], itype, hasFill ? ((double *)fillp[i])[0] : NA_REAL, nan_is_na, &vans[i], verbose); + nafillDouble(dx[i], inx[i], itype, hasFill ? ((double *)fillp[i])[0] : NA_REAL, nan_is_na, &vans[i], verbose, limit_val); } } break; case LGLSXP: case INTSXP : { - nafillInteger(ix[i], inx[i], itype, hasFill ? ((int32_t *)fillp[i])[0] : NA_INTEGER, &vans[i], verbose); + nafillInteger(ix[i], inx[i], itype, hasFill ? ((int32_t *)fillp[i])[0] : NA_INTEGER, &vans[i], verbose, limit_val); } break; case STRSXP : { - nafillString(sx[i], inx[i], itype, hasFill ? ((SEXP *)fillp[i])[0] : NA_STRING, &vans[i], verbose); + nafillString(sx[i], inx[i], itype, hasFill ? ((SEXP *)fillp[i])[0] : NA_STRING, &vans[i], verbose, limit_val); } break; } } From 5bbc47d2d156eae2f2ed9198c8204358380a6c9d Mon Sep 17 00:00:00 2001 From: venom1204 Date: Sat, 25 Jul 2026 14:35:09 +0000 Subject: [PATCH 02/21] added changes --- NEWS.md | 2 ++ R/shift.R | 14 ++++++++------ inst/tests/tests.Rraw | 23 +++++++++++++++++++---- man/nafill.Rd | 4 ++-- 4 files changed, 31 insertions(+), 12 deletions(-) diff --git a/NEWS.md b/NEWS.md index 17d2398d0e..cb75a2dcc6 100644 --- a/NEWS.md +++ b/NEWS.md @@ -36,6 +36,8 @@ 7. Rows can now be deleted by reference using `DT[i, .ROW := NULL]`, avoiding a full copy of the table for large row-removal operations, [#635](https://github.com/Rdatatable/data.table/issues/635). This has been one of data.table's most requested features. Target rows must be selected with the `i` expression, `by`/`keyby` are not supported, and keys/indices are cleared after deletion. The new experimental helper `setallocrow()` prepares columns for by-reference row operations. Thanks @arunsrinivasan for the feature request, @ben-schwen for the implementation, and @aitap for review and assistance. +8. `nafill()` and `setnafill()` gain a `limit` argument to restrict the maximum number of consecutive `NA` values filled during `locf` or `nocb` operations, [#7677](https://github.com/Rdatatable/data.table/issues/7677). Thanks to @jaynewton for the suggestion and @venom1204 for the PR. + ### BUG FIXES 1. `fread()` with `skip=0` and `(header=TRUE|FALSE)` no longer skips the first row when it has fewer fields than subsequent rows, [#7463](https://github.com/Rdatatable/data.table/issues/7463). Thanks @emayerhofer for the report and @ben-schwen for the fix. diff --git a/R/shift.R b/R/shift.R index c586bed0b6..468c432f45 100644 --- a/R/shift.R +++ b/R/shift.R @@ -26,16 +26,18 @@ shift = function(x, n=1L, fill, type=c("lag", "lead", "shift", "cyclic"), give.n ans } -nafill = function(x, type=c("const","locf","nocb"), fill=NA, nan=NA, limit=Inf) { +nafill = function(x, type=c("const", "locf", "nocb"), fill=NA, nan=NA, limit=Inf) { type = match.arg(type) - if (!is.numeric(limit) || length(limit) != 1L || limit < 0) - stopf("limit must be a non-negative scalar numeric or Inf") + if (!is.numeric(limit) || length(limit) != 1L || is.na(limit) || limit < 0) + stopf("limit must be a non-negative scalar numeric") + .Call(CnafillR, x, type, fill, nan_is_na(nan), FALSE, NULL, as.double(limit)) } -setnafill = function(x, type=c("const","locf","nocb"), fill=NA, nan=NA, cols=seq_along(x), limit=Inf) { +setnafill = function(x, type=c("const", "locf", "nocb"), fill=NA, nan=NA, cols=seq_along(x), limit=Inf) { type = match.arg(type) - if (!is.numeric(limit) || length(limit) != 1L || limit < 0) - stopf("limit must be a non-negative scalar numeric or Inf") + if (!is.numeric(limit) || length(limit) != 1L || is.na(limit) || limit < 0) + stopf("limit must be a non-negative scalar numeric") + invisible(.Call(CnafillR, x, type, fill, nan_is_na(nan), TRUE, cols, as.double(limit))) } diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index cac9ec511c..7443eee677 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21852,7 +21852,22 @@ test(2379.13, as.IDate("2020-01-01") + a, as.IDate(as.Date("2020-01-01") + a)) test(2379.14, as.IDate(d) - 1L, as.IDate(as.Date(d) - 1L)) # #7677: add limit argument to nafill() -test(2379.01, nafill(c(1, NA, NA, NA, 5), type="locf", limit=1), c(1, 1, NA, NA, 5)) -test(2379.02, nafill(c(1, NA, NA, 5, NA, NA, NA, 9), type="locf", limit=2), c(1, 1, 1, 5, 5, 5, NA, 9)) -test(2379.03, nafill(c(1, NA, NA, 4, NA), type="locf", limit=Inf), c(1, 1, 1, 4, 4)) -test(2379.04, nafill(c(NA, NA, 3, NA, NA), type="locf", limit=1), c(NA, NA, 3, 3, NA)) +test(2380.01, nafill(c(1, NA, NA, NA, 5), type="locf", limit=1), c(1, 1, NA, NA, 5)) +test(2380.02, nafill(c(1, NA, NA, 5, NA, NA, NA, 9), type="locf", limit=2), c(1, 1, 1, 5, 5, 5, NA, 9)) +test(2380.03, nafill(c(1, NA, NA, 4, NA), type="locf", limit=Inf), c(1, 1, 1, 4, 4)) +test(2380.04, nafill(c(NA, NA, 3, NA, NA), type="locf", limit=1), c(NA, NA, 3, 3, NA)) +test(2380.05, nafill(c(1, NA, NA, 5), type="nocb", limit=1), c(1, NA, 5, 5)) +test(2380.06, nafill(c(1L, NA, NA), type="locf", limit=1), c(1L, 1L, NA)) +test(2380.07, nafill(c("a", NA, NA), type="locf", limit=1), c("a", "a", NA)) +test(2380.08, nafill(c(1, NA, NA), type="locf", limit=0), c(1, NA, NA)) +dt = data.table(a=c(1, NA, NA)) +setnafill(dt, type="locf", limit=1) +test(2380.09, dt$a, c(1, 1, NA)) +test(2380.10, nafill(c(NA, 1), "locf", fill=0, limit=0), c(0, 1)) +test(2380.11, nafill(c(1, NA, NA), type="locf", limit=1.5), c(1, 1, NA)) +test(2380.12, nafill(c(1, NA, NA), type="const", fill=0, limit=1), c(1, 0, 0)) +test(2380.13, nafill(1:5, limit=-1), "limit must be a non-negative scalar numeric") +test(2380.14, nafill(1:5, limit=NA), "limit must be a non-negative scalar numeric") +test(2380.15, nafill(1:5, limit=NULL), "limit must be a non-negative scalar numeric") +test(2380.16, nafill(1:5, limit=c(1, 2)), "limit must be a non-negative scalar numeric") +test(2380.17, nafill(1:5, limit=1+2i), "limit must be a non-negative scalar numeric") diff --git a/man/nafill.Rd b/man/nafill.Rd index 57c2742b8e..03672daaa0 100644 --- a/man/nafill.Rd +++ b/man/nafill.Rd @@ -55,8 +55,8 @@ dt # limit= restricts the number of consecutive fills y = c(1, NA, NA, NA, 5) -nafill(y, "locf", limit=1) # Only fills the first NA -nafill(y, "locf", limit=2) # Fills the first two NAs +nafill(y, "locf", limit=1) # Only fills the first NA +nafill(y, "locf", limit=Inf) # Fills all NAs (default) } \seealso{ From 14af4f478ca58657b97c848413a254d0d826f359 Mon Sep 17 00:00:00 2001 From: venom1204 Date: Sat, 25 Jul 2026 14:48:07 +0000 Subject: [PATCH 03/21] .. --- R/shift.R | 4 ++-- inst/tests/tests.Rraw | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/R/shift.R b/R/shift.R index 468c432f45..fdaeb031d3 100644 --- a/R/shift.R +++ b/R/shift.R @@ -31,7 +31,7 @@ nafill = function(x, type=c("const", "locf", "nocb"), fill=NA, nan=NA, limit=Inf if (!is.numeric(limit) || length(limit) != 1L || is.na(limit) || limit < 0) stopf("limit must be a non-negative scalar numeric") - .Call(CnafillR, x, type, fill, nan_is_na(nan), FALSE, NULL, as.double(limit)) + .Call(CnafillR, x, type, fill, nan_is_na(nan), FALSE, NULL, as.double(floor(limit))) } setnafill = function(x, type=c("const", "locf", "nocb"), fill=NA, nan=NA, cols=seq_along(x), limit=Inf) { @@ -39,5 +39,5 @@ setnafill = function(x, type=c("const", "locf", "nocb"), fill=NA, nan=NA, cols=s if (!is.numeric(limit) || length(limit) != 1L || is.na(limit) || limit < 0) stopf("limit must be a non-negative scalar numeric") - invisible(.Call(CnafillR, x, type, fill, nan_is_na(nan), TRUE, cols, as.double(limit))) + invisible(.Call(CnafillR, x, type, fill, nan_is_na(nan), TRUE, cols, as.double(floor(limit)))) } diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 7443eee677..e45d1cd930 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21866,8 +21866,8 @@ test(2380.09, dt$a, c(1, 1, NA)) test(2380.10, nafill(c(NA, 1), "locf", fill=0, limit=0), c(0, 1)) test(2380.11, nafill(c(1, NA, NA), type="locf", limit=1.5), c(1, 1, NA)) test(2380.12, nafill(c(1, NA, NA), type="const", fill=0, limit=1), c(1, 0, 0)) -test(2380.13, nafill(1:5, limit=-1), "limit must be a non-negative scalar numeric") -test(2380.14, nafill(1:5, limit=NA), "limit must be a non-negative scalar numeric") -test(2380.15, nafill(1:5, limit=NULL), "limit must be a non-negative scalar numeric") -test(2380.16, nafill(1:5, limit=c(1, 2)), "limit must be a non-negative scalar numeric") -test(2380.17, nafill(1:5, limit=1+2i), "limit must be a non-negative scalar numeric") +test(2380.13, nafill(1:5, limit=-1), error="limit must be a non-negative scalar numeric") +test(2380.14, nafill(1:5, limit=NA), error="limit must be a non-negative scalar numeric") +test(2380.15, nafill(1:5, limit=NULL), error="limit must be a non-negative scalar numeric") +test(2380.16, nafill(1:5, limit=c(1, 2)), error="limit must be a non-negative scalar numeric") +test(2380.17, nafill(1:5, limit=1+2i), error="limit must be a non-negative scalar numeric") \ No newline at end of file From 7c59afe2def6daee408faf0f11b38c940dba6ab9 Mon Sep 17 00:00:00 2001 From: venom1204 Date: Sat, 25 Jul 2026 16:01:55 +0000 Subject: [PATCH 04/21] .. --- inst/tests/tests.Rraw | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index e45d1cd930..6fb6a8eda2 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21870,4 +21870,7 @@ test(2380.13, nafill(1:5, limit=-1), error="limit must be a non-negative scalar test(2380.14, nafill(1:5, limit=NA), error="limit must be a non-negative scalar numeric") test(2380.15, nafill(1:5, limit=NULL), error="limit must be a non-negative scalar numeric") test(2380.16, nafill(1:5, limit=c(1, 2)), error="limit must be a non-negative scalar numeric") -test(2380.17, nafill(1:5, limit=1+2i), error="limit must be a non-negative scalar numeric") \ No newline at end of file +test(2380.17, nafill(1:5, limit=1+2i), error="limit must be a non-negative scalar numeric") +test(2380.18, setnafill(data.table(a=1:5), limit=-1), error="limit must be a non-negative scalar numeric") +test(2380.19, nafill(c(1.1, NA, NA, NA, 4.4), type="nocb", limit=1), c(1.1, NA, NA, 4.4, 4.4)) +test(2380.20, nafill(c(1L, NA, NA, NA, 4L), type="nocb", limit=1), c(1L, NA, NA, 4L, 4L)) From 882189b4137ed5701e77ee24c46f94d31d1d555b Mon Sep 17 00:00:00 2001 From: venom1204 Date: Thu, 6 Aug 2026 19:09:21 +0000 Subject: [PATCH 05/21] updated changes --- NEWS.md | 2 +- R/shift.R | 2 - inst/tests/tests.Rraw | 6 +-- man/nafill.Rd | 3 +- src/data.table.h | 8 +-- src/nafill.c | 110 +++++++++++++----------------------------- 6 files changed, 43 insertions(+), 88 deletions(-) diff --git a/NEWS.md b/NEWS.md index e7854eb4c8..e10b706768 100644 --- a/NEWS.md +++ b/NEWS.md @@ -44,7 +44,7 @@ 11. `setorderv()` now accepts a named vector for the `order` argument. When provided, the names are used to identify the columns, allowing the `cols` argument to be omitted, [#6932](https://github.com/Rdatatable/data.table/issues/6932). Thanks to @MichaelChirico for the suggestion and @venom1204 for the implementation. -8. `nafill()` and `setnafill()` gain a `limit` argument to restrict the maximum number of consecutive `NA` values filled during `locf` or `nocb` operations, [#7677](https://github.com/Rdatatable/data.table/issues/7677). Thanks to @jaynewton for the suggestion and @venom1204 for the PR. +12. `nafill()` and `setnafill()` gain a `limit` argument to restrict the maximum number of consecutive `NA` values filled during `locf` or `nocb` operations, [#7677](https://github.com/Rdatatable/data.table/issues/7677). Thanks to @jaynewton for the suggestion and @venom1204 for the PR. ### BUG FIXES diff --git a/R/shift.R b/R/shift.R index fdaeb031d3..024081db6b 100644 --- a/R/shift.R +++ b/R/shift.R @@ -30,7 +30,6 @@ nafill = function(x, type=c("const", "locf", "nocb"), fill=NA, nan=NA, limit=Inf type = match.arg(type) if (!is.numeric(limit) || length(limit) != 1L || is.na(limit) || limit < 0) stopf("limit must be a non-negative scalar numeric") - .Call(CnafillR, x, type, fill, nan_is_na(nan), FALSE, NULL, as.double(floor(limit))) } @@ -38,6 +37,5 @@ setnafill = function(x, type=c("const", "locf", "nocb"), fill=NA, nan=NA, cols=s type = match.arg(type) if (!is.numeric(limit) || length(limit) != 1L || is.na(limit) || limit < 0) stopf("limit must be a non-negative scalar numeric") - invisible(.Call(CnafillR, x, type, fill, nan_is_na(nan), TRUE, cols, as.double(floor(limit)))) } diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 7e379a8c53..f217235f2b 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21964,7 +21964,7 @@ test(2380.07, nafill(c("a", NA, NA), type="locf", limit=1), c("a", "a", NA)) test(2380.08, nafill(c(1, NA, NA), type="locf", limit=0), c(1, NA, NA)) dt = data.table(a=c(1, NA, NA)) setnafill(dt, type="locf", limit=1) -test(2380.09, dt$a, c(1, 1, NA)) +test(2380.09, {dt=data.table(a=c(1,NA,NA)); setnafill(dt, type="locf", limit=1); dt$a}, c(1, 1, NA)) test(2380.10, nafill(c(NA, 1), "locf", fill=0, limit=0), c(0, 1)) test(2380.11, nafill(c(1, NA, NA), type="locf", limit=1.5), c(1, 1, NA)) test(2380.12, nafill(c(1, NA, NA), type="const", fill=0, limit=1), c(1, 0, 0)) @@ -21974,5 +21974,5 @@ test(2380.15, nafill(1:5, limit=NULL), error="limit must be a non-negative scala test(2380.16, nafill(1:5, limit=c(1, 2)), error="limit must be a non-negative scalar numeric") test(2380.17, nafill(1:5, limit=1+2i), error="limit must be a non-negative scalar numeric") test(2380.18, setnafill(data.table(a=1:5), limit=-1), error="limit must be a non-negative scalar numeric") -test(2380.19, nafill(c(1.1, NA, NA, NA, 4.4), type="nocb", limit=1), c(1.1, NA, NA, 4.4, 4.4)) -test(2380.20, nafill(c(1L, NA, NA, NA, 4L), type="nocb", limit=1), c(1L, NA, NA, 4L, 4L)) +test(2380.19, nafill(c(1.1, NA, NA, 4.4), type="nocb", limit=1), c(1.1, NA, 4.4, 4.4)) +test(2380.20, setnafill(data.table(a=1:5), limit=-1), error="limit must be a non-negative scalar numeric") diff --git a/man/nafill.Rd b/man/nafill.Rd index 03672daaa0..a863d82eda 100644 --- a/man/nafill.Rd +++ b/man/nafill.Rd @@ -19,7 +19,7 @@ setnafill(x, type=c("const", "locf", "nocb"), fill=NA, nan=NA, cols=seq_along(x) \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. } - \item{limit}{ The maximum number of consecutive \code{NA} values to fill. Must be a non-negative scalar numeric. Default is \code{Inf}. } + \item{limit}{The maximum number of consecutive \code{NA} values to fill. Must be a non-negative scalar numeric. Default is \code{Inf}. Fractional values are truncated via \code{floor}. This argument is ignored when \code{type="const"}.} } \details{ Supported types are \emph{logical}, \emph{integer}, \emph{double}, \emph{character}, and \emph{factor}, as well as classes built on top of these such as \code{Date}, \code{IDate}, and \code{POSIXct}. @@ -55,6 +55,7 @@ dt # limit= restricts the number of consecutive fills y = c(1, NA, NA, NA, 5) +nafill(c(NA, 1), "locf", fill=0, limit=0) # Result: 0, 1 (Boundary case) nafill(y, "locf", limit=1) # Only fills the first NA nafill(y, "locf", limit=Inf) # Fills all NAs (default) diff --git a/src/data.table.h b/src/data.table.h index d31ce3e9c5..4c4d112df0 100644 --- a/src/data.table.h +++ b/src/data.table.h @@ -311,10 +311,10 @@ SEXP copyAsGrowable(SEXP x); SEXP resizeVector(SEXP x, SEXP size); // nafill.c -void nafillDouble(double *x, uint_fast64_t nx, unsigned int type, double fill, bool nan_is_na, ans_t *ans, bool verbose, double limit); -void nafillInteger(int32_t *x, uint_fast64_t nx, unsigned int type, int32_t fill, ans_t *ans, bool verbose, double limit); -void nafillInteger64(int64_t *x, uint_fast64_t nx, unsigned int type, int64_t fill, ans_t *ans, bool verbose, double limit); -void nafillString(const SEXP *x, uint_fast64_t nx, unsigned int type, SEXP fill, ans_t *ans, bool verbose, double limit); +void nafillDouble(double *x, uint_fast64_t nx, unsigned int type, double fill, bool nan_is_na, ans_t *ans, bool verbose, uint_fast64_t limit); +void nafillInteger(int32_t *x, uint_fast64_t nx, unsigned int type, int32_t fill, ans_t *ans, bool verbose, uint_fast64_t limit); +void nafillInteger64(int64_t *x, uint_fast64_t nx, unsigned int type, int64_t fill, ans_t *ans, bool verbose, uint_fast64_t limit); +void nafillString(const SEXP *x, uint_fast64_t nx, unsigned int type, SEXP fill, ans_t *ans, bool verbose, uint_fast64_t limit); SEXP nafillR(SEXP obj, SEXP type, SEXP fill, SEXP nan_is_na_arg, SEXP inplace, SEXP cols, SEXP limit); // between.c diff --git a/src/nafill.c b/src/nafill.c index 197fe954cb..ac46735ebc 100644 --- a/src/nafill.c +++ b/src/nafill.c @@ -1,66 +1,43 @@ #include "data.table.h" -void nafillDouble(double *x, uint_fast64_t nx, unsigned int type, double fill, bool nan_is_na, ans_t *ans, bool verbose, double limit) { +void nafillDouble(double *x, uint_fast64_t nx, unsigned int type, double fill, bool nan_is_na, ans_t *ans, bool verbose, uint_fast64_t limit) { double tic=0.0; if (verbose) tic = omp_get_wtime(); if (type==0) { // const - if (nan_is_na) { - for (uint_fast64_t i=0; idbl_v[i] = ISNAN(x[i]) ? fill : x[i]; - } - } else { - for (uint_fast64_t i=0; idbl_v[i] = ISNA(x[i]) ? fill : x[i]; - } + for (uint_fast64_t i=0; idbl_v[i] = is_na ? fill : x[i]; } } else if (type==1) { // locf uint_fast64_t fills = 0; - if (nan_is_na) { - ans->dbl_v[0] = ISNAN(x[0]) ? fill : x[0]; - if (ISNAN(x[0])) fills = 1; - for (uint_fast64_t i=1; idbl_v[i] = ans->dbl_v[i-1]; fills++; } - else ans->dbl_v[i] = x[i]; - } else { ans->dbl_v[i] = x[i]; fills = 0; } - } - } else { - ans->dbl_v[0] = ISNA(x[0]) ? fill : x[0]; - if (ISNA(x[0])) fills = 1; - for (uint_fast64_t i=1; idbl_v[i] = ans->dbl_v[i-1]; fills++; } - else ans->dbl_v[i] = x[i]; - } else { ans->dbl_v[i] = x[i]; fills = 0; } - } + bool is_na = nan_is_na ? ISNAN(x[0]) : ISNA(x[0]); + ans->dbl_v[0] = is_na ? fill : x[0]; + if (is_na) fills = 1; + for (uint_fast64_t i=1; idbl_v[i] = ans->dbl_v[i-1]; fills++; } + else ans->dbl_v[i] = x[i]; + } else { ans->dbl_v[i] = x[i]; fills = 0; } } } else if (type==2) { // nocb uint_fast64_t fills = 0; - if (nan_is_na) { - ans->dbl_v[nx-1] = ISNAN(x[nx-1]) ? fill : x[nx-1]; - if (ISNAN(x[nx-1])) fills = 1; - for (int_fast64_t i=nx-2; i>=0; i--) { - if (ISNAN(x[i])) { - if (fills < limit) { ans->dbl_v[i] = ans->dbl_v[i+1]; fills++; } - else ans->dbl_v[i] = x[i]; - } else { ans->dbl_v[i] = x[i]; fills = 0; } - } - } else { - ans->dbl_v[nx-1] = ISNA(x[nx-1]) ? fill : x[nx-1]; - if (ISNA(x[nx-1])) fills = 1; - for (int_fast64_t i=nx-2; i>=0; i--) { - if (ISNA(x[i])) { - if (fills < limit) { ans->dbl_v[i] = ans->dbl_v[i+1]; fills++; } - else ans->dbl_v[i] = x[i]; - } else { ans->dbl_v[i] = x[i]; fills = 0; } - } + bool is_na = nan_is_na ? ISNAN(x[nx-1]) : ISNA(x[nx-1]); + ans->dbl_v[nx-1] = is_na ? fill : x[nx-1]; + if (is_na) fills = 1; + for (int_fast64_t i=nx-2; i>=0; i--) { + is_na = nan_is_na ? ISNAN(x[i]) : ISNA(x[i]); + if (is_na) { + if (fills < limit) { ans->dbl_v[i] = ans->dbl_v[i+1]; fills++; } + else ans->dbl_v[i] = x[i]; + } else { ans->dbl_v[i] = x[i]; fills = 0; } } } if (verbose) snprintf(ans->message[0], 500, _("%s: took %.3fs\n"), __func__, omp_get_wtime()-tic); } -void nafillInteger(int32_t *x, uint_fast64_t nx, unsigned int type, int32_t fill, ans_t *ans, bool verbose, double limit) { +void nafillInteger(int32_t *x, uint_fast64_t nx, unsigned int type, int32_t fill, ans_t *ans, bool verbose, uint_fast64_t limit) { double tic=0.0; if (verbose) tic = omp_get_wtime(); @@ -92,7 +69,7 @@ void nafillInteger(int32_t *x, uint_fast64_t nx, unsigned int type, int32_t fill if (verbose) snprintf(ans->message[0], 500, _("%s: took %.3fs\n"), __func__, omp_get_wtime()-tic); } -void nafillInteger64(int64_t *x, uint_fast64_t nx, unsigned int type, int64_t fill, ans_t *ans, bool verbose, double limit) { +void nafillInteger64(int64_t *x, uint_fast64_t nx, unsigned int type, int64_t fill, ans_t *ans, bool verbose, uint_fast64_t limit) { double tic=0.0; if (verbose) tic = omp_get_wtime(); @@ -125,7 +102,7 @@ void nafillInteger64(int64_t *x, uint_fast64_t nx, unsigned int type, int64_t fi snprintf(ans->message[0], 500, _("%s: took %.3fs\n"), __func__, omp_get_wtime()-tic); } -void nafillString(const SEXP *x, uint_fast64_t nx, unsigned int type, SEXP fill, ans_t *ans, bool verbose, double limit) { +void nafillString(const SEXP *x, uint_fast64_t nx, unsigned int type, SEXP fill, ans_t *ans, bool verbose, uint_fast64_t limit) { double tic=0.0; if (verbose) tic = omp_get_wtime(); @@ -176,7 +153,8 @@ SEXP nafillR(SEXP obj, SEXP type, SEXP fill, SEXP nan_is_na_arg, SEXP inplace, S if (verbose) tic = omp_get_wtime(); - double limit_val = REAL(limit)[0]; + const double limit_d = REAL(limit)[0]; + const uint_fast64_t limit_n = !R_FINITE(limit_d) || limit_d >= (double)UINT_FAST64_MAX ? UINT_FAST64_MAX : (uint_fast64_t)limit_d; bool copy = !LOGICAL(inplace)[0]; if (!IS_TRUE_OR_FALSE(nan_is_na_arg)) @@ -236,21 +214,7 @@ SEXP nafillR(SEXP obj, SEXP type, SEXP fill, SEXP nan_is_na_arg, SEXP inplace, S for (R_len_t i=0; i Date: Thu, 6 Aug 2026 19:31:28 +0000 Subject: [PATCH 06/21] .. --- inst/tests/tests.Rraw | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index f217235f2b..0a093973b9 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21954,25 +21954,25 @@ DT = data.table(a=c(2,1,2), b=3:1) test(2385.06, setorderv(copy(DT), order=c(a=1L)), setorderv(copy(DT), cols="a", order=1L)) # #7677: add limit argument to nafill() -test(2380.01, nafill(c(1, NA, NA, NA, 5), type="locf", limit=1), c(1, 1, NA, NA, 5)) -test(2380.02, nafill(c(1, NA, NA, 5, NA, NA, NA, 9), type="locf", limit=2), c(1, 1, 1, 5, 5, 5, NA, 9)) -test(2380.03, nafill(c(1, NA, NA, 4, NA), type="locf", limit=Inf), c(1, 1, 1, 4, 4)) -test(2380.04, nafill(c(NA, NA, 3, NA, NA), type="locf", limit=1), c(NA, NA, 3, 3, NA)) -test(2380.05, nafill(c(1, NA, NA, 5), type="nocb", limit=1), c(1, NA, 5, 5)) -test(2380.06, nafill(c(1L, NA, NA), type="locf", limit=1), c(1L, 1L, NA)) -test(2380.07, nafill(c("a", NA, NA), type="locf", limit=1), c("a", "a", NA)) -test(2380.08, nafill(c(1, NA, NA), type="locf", limit=0), c(1, NA, NA)) +test(2386.01, nafill(c(1, NA, NA, NA, 5), type="locf", limit=1), c(1, 1, NA, NA, 5)) +test(2386.02, nafill(c(1, NA, NA, 5, NA, NA, NA, 9), type="locf", limit=2), c(1, 1, 1, 5, 5, 5, NA, 9)) +test(2386.03, nafill(c(1, NA, NA, 4, NA), type="locf", limit=Inf), c(1, 1, 1, 4, 4)) +test(2386.04, nafill(c(NA, NA, 3, NA, NA), type="locf", limit=1), c(NA, NA, 3, 3, NA)) +test(2386.05, nafill(c(1, NA, NA, 5), type="nocb", limit=1), c(1, NA, 5, 5)) +test(2386.06, nafill(c(1L, NA, NA), type="locf", limit=1), c(1L, 1L, NA)) +test(2386.07, nafill(c("a", NA, NA), type="locf", limit=1), c("a", "a", NA)) +test(2386.08, nafill(c(1, NA, NA), type="locf", limit=0), c(1, NA, NA)) dt = data.table(a=c(1, NA, NA)) setnafill(dt, type="locf", limit=1) -test(2380.09, {dt=data.table(a=c(1,NA,NA)); setnafill(dt, type="locf", limit=1); dt$a}, c(1, 1, NA)) -test(2380.10, nafill(c(NA, 1), "locf", fill=0, limit=0), c(0, 1)) -test(2380.11, nafill(c(1, NA, NA), type="locf", limit=1.5), c(1, 1, NA)) -test(2380.12, nafill(c(1, NA, NA), type="const", fill=0, limit=1), c(1, 0, 0)) -test(2380.13, nafill(1:5, limit=-1), error="limit must be a non-negative scalar numeric") -test(2380.14, nafill(1:5, limit=NA), error="limit must be a non-negative scalar numeric") -test(2380.15, nafill(1:5, limit=NULL), error="limit must be a non-negative scalar numeric") -test(2380.16, nafill(1:5, limit=c(1, 2)), error="limit must be a non-negative scalar numeric") -test(2380.17, nafill(1:5, limit=1+2i), error="limit must be a non-negative scalar numeric") -test(2380.18, setnafill(data.table(a=1:5), limit=-1), error="limit must be a non-negative scalar numeric") -test(2380.19, nafill(c(1.1, NA, NA, 4.4), type="nocb", limit=1), c(1.1, NA, 4.4, 4.4)) -test(2380.20, setnafill(data.table(a=1:5), limit=-1), error="limit must be a non-negative scalar numeric") +test(2386.09, {dt=data.table(a=c(1,NA,NA)); setnafill(dt, type="locf", limit=1); dt$a}, c(1, 1, NA)) +test(2386.10, nafill(c(NA, 1), "locf", fill=0, limit=0), c(0, 1)) +test(2386.11, nafill(c(1, NA, NA), type="locf", limit=1.5), c(1, 1, NA)) +test(2386.12, nafill(c(1, NA, NA), type="const", fill=0, limit=1), c(1, 0, 0)) +test(2386.13, nafill(1:5, limit=-1), error="limit must be a non-negative scalar numeric") +test(2386.14, nafill(1:5, limit=NA), error="limit must be a non-negative scalar numeric") +test(2386.15, nafill(1:5, limit=NULL), error="limit must be a non-negative scalar numeric") +test(2386.16, nafill(1:5, limit=c(1, 2)), error="limit must be a non-negative scalar numeric") +test(2386.17, nafill(1:5, limit=1+2i), error="limit must be a non-negative scalar numeric") +test(2386.18, setnafill(data.table(a=1:5), limit=-1), error="limit must be a non-negative scalar numeric") +test(2386.19, nafill(c(1.1, NA, NA, 4.4), type="nocb", limit=1), c(1.1, NA, 4.4, 4.4)) +test(2386.20, setnafill(data.table(a=1:5), limit=-1), error="limit must be a non-negative scalar numeric") From 113076cf92799cbefd7e6258603a9b5b7b9df95e Mon Sep 17 00:00:00 2001 From: venom1204 Date: Mon, 10 Aug 2026 21:09:35 +0000 Subject: [PATCH 07/21] .. --- NEWS.md | 2 +- inst/tests/tests.Rraw | 34 ++++++----------- src/nafill.c | 88 ++++++++++++++++++++++++++----------------- 3 files changed, 67 insertions(+), 57 deletions(-) diff --git a/NEWS.md b/NEWS.md index 2b236bb6c8..199464db18 100644 --- a/NEWS.md +++ b/NEWS.md @@ -48,7 +48,7 @@ 13. `setnafill()` now accepts a logical vector for the `cols` argument, which must be the same length as the number of columns in `x`, [#4113](https://github.com/Rdatatable/data.table/issues/4113). Thanks to @MichaelChirico for the suggestion and @venom1204 for the PR. -12. `nafill()` and `setnafill()` gain a `limit` argument to restrict the maximum number of consecutive `NA` values filled during `locf` or `nocb` operations, [#7677](https://github.com/Rdatatable/data.table/issues/7677). Thanks to @jaynewton for the suggestion and @venom1204 for the PR. +14. `nafill()` and `setnafill()` gain a `limit` argument to restrict the maximum number of consecutive `NA` values filled during `locf` or `nocb` operations, [#7677](https://github.com/Rdatatable/data.table/issues/7677). Thanks to @jaynewton for the suggestion and @venom1204 for the PR. ### BUG FIXES diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 8f7c1ea92e..c7e6ed9c54 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21987,25 +21987,15 @@ test(2387.05, setnafill(copy(DT3), type="locf", cols=c(TRUE,NA,FALSE)), error="' test(2387.06, setnafill(copy(DT3), type="locf", cols=c(TRUE,FALSE)), error="'cols' is a logical vector of length 2 but there are 3 columns") # #7677: add limit argument to nafill() -test(2386.01, nafill(c(1, NA, NA, NA, 5), type="locf", limit=1), c(1, 1, NA, NA, 5)) -test(2386.02, nafill(c(1, NA, NA, 5, NA, NA, NA, 9), type="locf", limit=2), c(1, 1, 1, 5, 5, 5, NA, 9)) -test(2386.03, nafill(c(1, NA, NA, 4, NA), type="locf", limit=Inf), c(1, 1, 1, 4, 4)) -test(2386.04, nafill(c(NA, NA, 3, NA, NA), type="locf", limit=1), c(NA, NA, 3, 3, NA)) -test(2386.05, nafill(c(1, NA, NA, 5), type="nocb", limit=1), c(1, NA, 5, 5)) -test(2386.06, nafill(c(1L, NA, NA), type="locf", limit=1), c(1L, 1L, NA)) -test(2386.07, nafill(c("a", NA, NA), type="locf", limit=1), c("a", "a", NA)) -test(2386.08, nafill(c(1, NA, NA), type="locf", limit=0), c(1, NA, NA)) -dt = data.table(a=c(1, NA, NA)) -setnafill(dt, type="locf", limit=1) -test(2386.09, {dt=data.table(a=c(1,NA,NA)); setnafill(dt, type="locf", limit=1); dt$a}, c(1, 1, NA)) -test(2386.10, nafill(c(NA, 1), "locf", fill=0, limit=0), c(0, 1)) -test(2386.11, nafill(c(1, NA, NA), type="locf", limit=1.5), c(1, 1, NA)) -test(2386.12, nafill(c(1, NA, NA), type="const", fill=0, limit=1), c(1, 0, 0)) -test(2386.13, nafill(1:5, limit=-1), error="limit must be a non-negative scalar numeric") -test(2386.14, nafill(1:5, limit=NA), error="limit must be a non-negative scalar numeric") -test(2386.15, nafill(1:5, limit=NULL), error="limit must be a non-negative scalar numeric") -test(2386.16, nafill(1:5, limit=c(1, 2)), error="limit must be a non-negative scalar numeric") -test(2386.17, nafill(1:5, limit=1+2i), error="limit must be a non-negative scalar numeric") -test(2386.18, setnafill(data.table(a=1:5), limit=-1), error="limit must be a non-negative scalar numeric") -test(2386.19, nafill(c(1.1, NA, NA, 4.4), type="nocb", limit=1), c(1.1, NA, 4.4, 4.4)) -test(2386.20, setnafill(data.table(a=1:5), limit=-1), error="limit must be a non-negative scalar numeric") +test(2388.01, list(nafill(c(1, NA, NA, NA, 5), type="locf", limit=1), nafill(c(1, NA, NA, 5, NA, NA, NA, 9), type="locf", limit=2), nafill(c(1, NA, NA, 4, NA), type="locf", limit=Inf)), list(c(1, 1, NA, NA, 5), c(1, 1, 1, 5, 5, 5, NA, 9), c(1, 1, 1, 4, 4))) +test(2388.02, list(nafill(c(1L, NA, NA), type="locf", limit=1), nafill(c("a", NA, NA), type="locf", limit=1)), list(c(1L, 1L, NA), c("a", "a", NA))) +test(2388.03, nafill(c(NA, NA, 3, NA, NA), type="locf", limit=1), c(NA, NA, 3, 3, NA)) +test(2388.04, nafill(c(1, NA, NA, 5), type="nocb", limit=1), c(1, NA, 5, 5)) +test(2388.05, setnafill(data.table(a=c(1, NA, NA)), type="locf", limit=1), data.table(a=c(1, 1, NA))) +test(2388.06, nafill(c(1, NA, NA), type="locf", limit=0), c(1, NA, NA)) +test(2388.07, nafill(c(NA, 1), "locf", fill=0, limit=0), c(0, 1)) +test(2388.08, nafill(c(1, NA, NA), type="locf", limit=1.5), c(1, 1, NA)) +test(2388.09, nafill(c(1, NA, NA), type="const", fill=0, limit=1), c(1, 0, 0)) +lims = list(-1, NA, NaN, NULL, c(1, 2), 1+2i); for (i in seq_along(lims)) { test(2388.09 + i / 100, nafill(1:5, limit=lims[[i]]), error="limit must be a non-negative scalar numeric") } +test(2388.16, setnafill(data.table(a=1:5), limit=-1), error="limit must be a non-negative scalar numeric") +test(2388.17, nafill(c(1.1, NA, NA, 4.4), type="nocb", limit=1), c(1.1, NA, 4.4, 4.4)) diff --git a/src/nafill.c b/src/nafill.c index ac46735ebc..f8afc8ba39 100644 --- a/src/nafill.c +++ b/src/nafill.c @@ -2,40 +2,57 @@ void nafillDouble(double *x, uint_fast64_t nx, unsigned int type, double fill, bool nan_is_na, ans_t *ans, bool verbose, uint_fast64_t limit) { double tic=0.0; - if (verbose) - tic = omp_get_wtime(); + if (verbose) tic = omp_get_wtime(); if (type==0) { // const - for (uint_fast64_t i=0; idbl_v[i] = is_na ? fill : x[i]; + if (nan_is_na) { + for (uint_fast64_t i=0; idbl_v[i] = ISNAN(x[i]) ? fill : x[i]; + } else { + for (uint_fast64_t i=0; idbl_v[i] = ISNA(x[i]) ? fill : x[i]; } } else if (type==1) { // locf uint_fast64_t fills = 0; - bool is_na = nan_is_na ? ISNAN(x[0]) : ISNA(x[0]); - ans->dbl_v[0] = is_na ? fill : x[0]; - if (is_na) fills = 1; - for (uint_fast64_t i=1; idbl_v[i] = ans->dbl_v[i-1]; fills++; } - else ans->dbl_v[i] = x[i]; - } else { ans->dbl_v[i] = x[i]; fills = 0; } + if (nan_is_na) { + ans->dbl_v[0] = ISNAN(x[0]) ? fill : x[0]; + if (ISNAN(x[0])) fills = 1; + for (uint_fast64_t i=1; idbl_v[i] = ans->dbl_v[i-1]; fills++; } + else ans->dbl_v[i] = x[i]; + } else { ans->dbl_v[i] = x[i]; fills = 0; } + } + } else { + ans->dbl_v[0] = ISNA(x[0]) ? fill : x[0]; + if (ISNA(x[0])) fills = 1; + for (uint_fast64_t i=1; idbl_v[i] = ans->dbl_v[i-1]; fills++; } + else ans->dbl_v[i] = x[i]; + } else { ans->dbl_v[i] = x[i]; fills = 0; } + } } } else if (type==2) { // nocb uint_fast64_t fills = 0; - bool is_na = nan_is_na ? ISNAN(x[nx-1]) : ISNA(x[nx-1]); - ans->dbl_v[nx-1] = is_na ? fill : x[nx-1]; - if (is_na) fills = 1; - for (int_fast64_t i=nx-2; i>=0; i--) { - is_na = nan_is_na ? ISNAN(x[i]) : ISNA(x[i]); - if (is_na) { - if (fills < limit) { ans->dbl_v[i] = ans->dbl_v[i+1]; fills++; } - else ans->dbl_v[i] = x[i]; - } else { ans->dbl_v[i] = x[i]; fills = 0; } + if (nan_is_na) { + ans->dbl_v[nx-1] = ISNAN(x[nx-1]) ? fill : x[nx-1]; + if (ISNAN(x[nx-1])) fills = 1; + for (int_fast64_t i=nx-2; i>=0; i--) { + if (ISNAN(x[i])) { + if (fills < limit) { ans->dbl_v[i] = ans->dbl_v[i+1]; fills++; } + else ans->dbl_v[i] = x[i]; + } else { ans->dbl_v[i] = x[i]; fills = 0; } + } + } else { + ans->dbl_v[nx-1] = ISNA(x[nx-1]) ? fill : x[nx-1]; + if (ISNA(x[nx-1])) fills = 1; + for (int_fast64_t i=nx-2; i>=0; i--) { + if (ISNA(x[i])) { + if (fills < limit) { ans->dbl_v[i] = ans->dbl_v[i+1]; fills++; } + else ans->dbl_v[i] = x[i]; + } else { ans->dbl_v[i] = x[i]; fills = 0; } + } } } - if (verbose) - snprintf(ans->message[0], 500, _("%s: took %.3fs\n"), __func__, omp_get_wtime()-tic); + if (verbose) snprintf(ans->message[0], 500, _("%s: took %.3fs\n"), __func__, omp_get_wtime()-tic); } void nafillInteger(int32_t *x, uint_fast64_t nx, unsigned int type, int32_t fill, ans_t *ans, bool verbose, uint_fast64_t limit) { double tic=0.0; @@ -145,20 +162,15 @@ void nafillString(const SEXP *x, uint_fast64_t nx, unsigned int type, SEXP fill, SEXP nafillR(SEXP obj, SEXP type, SEXP fill, SEXP nan_is_na_arg, SEXP inplace, SEXP cols, SEXP limit) { int protecti=0; const bool verbose = GetVerbose(); - - if (!xlength(obj)) - return(obj); - + if (!xlength(obj)) return(obj); double tic=0.0; - if (verbose) - tic = omp_get_wtime(); + if (verbose) tic = omp_get_wtime(); const double limit_d = REAL(limit)[0]; const uint_fast64_t limit_n = !R_FINITE(limit_d) || limit_d >= (double)UINT_FAST64_MAX ? UINT_FAST64_MAX : (uint_fast64_t)limit_d; bool copy = !LOGICAL(inplace)[0]; - if (!IS_TRUE_OR_FALSE(nan_is_na_arg)) - error(_("'%s' must be TRUE or FALSE"), "nan_is_na"); // # nocov + if (!IS_TRUE_OR_FALSE(nan_is_na_arg)) error(_("'%s' must be TRUE or FALSE"), "nan_is_na"); bool nan_is_na = LOGICAL(nan_is_na_arg)[0]; SEXP x = R_NilValue; @@ -224,7 +236,15 @@ SEXP nafillR(SEXP obj, SEXP type, SEXP fill, SEXP nan_is_na_arg, SEXP inplace, S } } - unsigned int itype = !strcmp(CHAR(STRING_ELT(type, 0)), "const") ? 0 : (!strcmp(CHAR(STRING_ELT(type, 0)), "locf") ? 1 : 2); + unsigned int itype=-1; + if (!strcmp(CHAR(STRING_ELT(type, 0)), "const")) + itype = 0; + else if (!strcmp(CHAR(STRING_ELT(type, 0)), "locf")) + itype = 1; + else if (!strcmp(CHAR(STRING_ELT(type, 0)), "nocb")) + itype = 2; + else + internal_error(__func__, "invalid %s argument in %s function should have been caught earlier", "type", "nafillR"); bool hasFill = !isLogical(fill) || LOGICAL(fill)[0]!=NA_LOGICAL; bool *isInt64 = (bool *)R_alloc(nx, sizeof(*isInt64)); From 5026744ac581a7c327d3304059a87410ac4e84c5 Mon Sep 17 00:00:00 2001 From: venom1204 Date: Mon, 10 Aug 2026 21:19:44 +0000 Subject: [PATCH 08/21] added test for that uncovered line --- inst/tests/tests.Rraw | 2 ++ 1 file changed, 2 insertions(+) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index c7e6ed9c54..f0324054eb 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21999,3 +21999,5 @@ test(2388.09, nafill(c(1, NA, NA), type="const", fill=0, limit=1), c(1, 0, 0)) lims = list(-1, NA, NaN, NULL, c(1, 2), 1+2i); for (i in seq_along(lims)) { test(2388.09 + i / 100, nafill(1:5, limit=lims[[i]]), error="limit must be a non-negative scalar numeric") } test(2388.16, setnafill(data.table(a=1:5), limit=-1), error="limit must be a non-negative scalar numeric") test(2388.17, nafill(c(1.1, NA, NA, 4.4), type="nocb", limit=1), c(1.1, NA, 4.4, 4.4)) +test(2388.18, nafill(c(1.1, NA, NA), type="locf", limit=1, nan=NaN), c(1.1, 1.1, NA)) +test(2388.19, nafill(c(1.1, NA, NA), type="nocb", limit=1, nan=NaN), c(NA, 1.1, 1.1)) From d86967d481d87133466570a77456e85a6fb7e2fb Mon Sep 17 00:00:00 2001 From: venom1204 Date: Tue, 11 Aug 2026 16:31:10 +0000 Subject: [PATCH 09/21] .. --- inst/tests/tests.Rraw | 1 - 1 file changed, 1 deletion(-) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index f0324054eb..3234d59422 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -22000,4 +22000,3 @@ lims = list(-1, NA, NaN, NULL, c(1, 2), 1+2i); for (i in seq_along(lims)) { test test(2388.16, setnafill(data.table(a=1:5), limit=-1), error="limit must be a non-negative scalar numeric") test(2388.17, nafill(c(1.1, NA, NA, 4.4), type="nocb", limit=1), c(1.1, NA, 4.4, 4.4)) test(2388.18, nafill(c(1.1, NA, NA), type="locf", limit=1, nan=NaN), c(1.1, 1.1, NA)) -test(2388.19, nafill(c(1.1, NA, NA), type="nocb", limit=1, nan=NaN), c(NA, 1.1, 1.1)) From 005159d73fddfc0c02cb5f5fb42b474b6402b6a3 Mon Sep 17 00:00:00 2001 From: Benjamin Schwendinger Date: Thu, 20 Aug 2026 09:49:42 +0200 Subject: [PATCH 10/21] clean up nocov --- src/nafill.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nafill.c b/src/nafill.c index f8afc8ba39..c4fcd37d07 100644 --- a/src/nafill.c +++ b/src/nafill.c @@ -244,7 +244,7 @@ SEXP nafillR(SEXP obj, SEXP type, SEXP fill, SEXP nan_is_na_arg, SEXP inplace, S else if (!strcmp(CHAR(STRING_ELT(type, 0)), "nocb")) itype = 2; else - internal_error(__func__, "invalid %s argument in %s function should have been caught earlier", "type", "nafillR"); + internal_error(__func__, "invalid %s argument in %s function should have been caught earlier", "type", "nafillR"); // # nocov bool hasFill = !isLogical(fill) || LOGICAL(fill)[0]!=NA_LOGICAL; bool *isInt64 = (bool *)R_alloc(nx, sizeof(*isInt64)); From dd74307fe619308202f95f9e6ab57c4d738465be Mon Sep 17 00:00:00 2001 From: Benjamin Schwendinger Date: Thu, 20 Aug 2026 09:53:32 +0200 Subject: [PATCH 11/21] clean up ternary shenanigans --- src/nafill.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/nafill.c b/src/nafill.c index c4fcd37d07..c436ce6cfd 100644 --- a/src/nafill.c +++ b/src/nafill.c @@ -226,7 +226,21 @@ SEXP nafillR(SEXP obj, SEXP type, SEXP fill, SEXP nan_is_na_arg, SEXP inplace, S for (R_len_t i=0; i Date: Thu, 20 Aug 2026 09:55:00 +0200 Subject: [PATCH 12/21] clean up nocov spill --- src/nafill.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/nafill.c b/src/nafill.c index c436ce6cfd..fa6d0fa238 100644 --- a/src/nafill.c +++ b/src/nafill.c @@ -170,7 +170,8 @@ SEXP nafillR(SEXP obj, SEXP type, SEXP fill, SEXP nan_is_na_arg, SEXP inplace, S const uint_fast64_t limit_n = !R_FINITE(limit_d) || limit_d >= (double)UINT_FAST64_MAX ? UINT_FAST64_MAX : (uint_fast64_t)limit_d; bool copy = !LOGICAL(inplace)[0]; - if (!IS_TRUE_OR_FALSE(nan_is_na_arg)) error(_("'%s' must be TRUE or FALSE"), "nan_is_na"); + if (!IS_TRUE_OR_FALSE(nan_is_na_arg)) + error(_("'%s' must be TRUE or FALSE"), "nan_is_na"); // # nocov bool nan_is_na = LOGICAL(nan_is_na_arg)[0]; SEXP x = R_NilValue; From 15d574202015891cc1630a30c1a5118aab1c7b71 Mon Sep 17 00:00:00 2001 From: Benjamin Schwendinger Date: Thu, 20 Aug 2026 09:55:52 +0200 Subject: [PATCH 13/21] clean up unnecessary change --- src/nafill.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/nafill.c b/src/nafill.c index fa6d0fa238..c2de30f0d5 100644 --- a/src/nafill.c +++ b/src/nafill.c @@ -162,9 +162,13 @@ void nafillString(const SEXP *x, uint_fast64_t nx, unsigned int type, SEXP fill, SEXP nafillR(SEXP obj, SEXP type, SEXP fill, SEXP nan_is_na_arg, SEXP inplace, SEXP cols, SEXP limit) { int protecti=0; const bool verbose = GetVerbose(); - if (!xlength(obj)) return(obj); + + if (!xlength(obj)) + return(obj); + double tic=0.0; - if (verbose) tic = omp_get_wtime(); + if (verbose) + tic = omp_get_wtime(); const double limit_d = REAL(limit)[0]; const uint_fast64_t limit_n = !R_FINITE(limit_d) || limit_d >= (double)UINT_FAST64_MAX ? UINT_FAST64_MAX : (uint_fast64_t)limit_d; From 0c15ebebde5eb655518dbd798cf2668e3f9eaa18 Mon Sep 17 00:00:00 2001 From: Benjamin Schwendinger Date: Thu, 20 Aug 2026 09:56:22 +0200 Subject: [PATCH 14/21] clean up spill --- src/nafill.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/nafill.c b/src/nafill.c index c2de30f0d5..f5e74f6753 100644 --- a/src/nafill.c +++ b/src/nafill.c @@ -2,7 +2,8 @@ void nafillDouble(double *x, uint_fast64_t nx, unsigned int type, double fill, bool nan_is_na, ans_t *ans, bool verbose, uint_fast64_t limit) { double tic=0.0; - if (verbose) tic = omp_get_wtime(); + if (verbose) + tic = omp_get_wtime(); if (type==0) { // const if (nan_is_na) { for (uint_fast64_t i=0; idbl_v[i] = ISNAN(x[i]) ? fill : x[i]; From bf969edbfad5fefa1a44a5c394a1e54058b213c4 Mon Sep 17 00:00:00 2001 From: Benjamin Schwendinger Date: Thu, 20 Aug 2026 09:57:25 +0200 Subject: [PATCH 15/21] clean up spill --- src/nafill.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/nafill.c b/src/nafill.c index f5e74f6753..15a96032f0 100644 --- a/src/nafill.c +++ b/src/nafill.c @@ -6,9 +6,13 @@ void nafillDouble(double *x, uint_fast64_t nx, unsigned int type, double fill, b tic = omp_get_wtime(); if (type==0) { // const if (nan_is_na) { - for (uint_fast64_t i=0; idbl_v[i] = ISNAN(x[i]) ? fill : x[i]; + for (uint_fast64_t i=0; idbl_v[i] = ISNAN(x[i]) ? fill : x[i]; + } } else { - for (uint_fast64_t i=0; idbl_v[i] = ISNA(x[i]) ? fill : x[i]; + for (uint_fast64_t i=0; idbl_v[i] = ISNA(x[i]) ? fill : x[i]; + } } } else if (type==1) { // locf uint_fast64_t fills = 0; From b072155f3aed36ea6a8809e2e46840d77987cb32 Mon Sep 17 00:00:00 2001 From: Benjamin Schwendinger Date: Thu, 20 Aug 2026 10:20:52 +0200 Subject: [PATCH 16/21] remove spill --- src/nafill.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/nafill.c b/src/nafill.c index 15a96032f0..166254a2b6 100644 --- a/src/nafill.c +++ b/src/nafill.c @@ -57,7 +57,8 @@ void nafillDouble(double *x, uint_fast64_t nx, unsigned int type, double fill, b } } } - if (verbose) snprintf(ans->message[0], 500, _("%s: took %.3fs\n"), __func__, omp_get_wtime()-tic); + if (verbose) + snprintf(ans->message[0], 500, _("%s: took %.3fs\n"), __func__, omp_get_wtime()-tic); } void nafillInteger(int32_t *x, uint_fast64_t nx, unsigned int type, int32_t fill, ans_t *ans, bool verbose, uint_fast64_t limit) { double tic=0.0; From 8e3a75fa25d0a18a75812e90605ed3b252186e4b Mon Sep 17 00:00:00 2001 From: Benjamin Schwendinger Date: Thu, 20 Aug 2026 10:31:48 +0200 Subject: [PATCH 17/21] add limit for const --- NEWS.md | 2 +- inst/tests/tests.Rraw | 4 +++- man/nafill.Rd | 3 ++- src/nafill.c | 31 +++++++++++++++++++++++++------ 4 files changed, 31 insertions(+), 9 deletions(-) diff --git a/NEWS.md b/NEWS.md index 199464db18..4630c2aabd 100644 --- a/NEWS.md +++ b/NEWS.md @@ -48,7 +48,7 @@ 13. `setnafill()` now accepts a logical vector for the `cols` argument, which must be the same length as the number of columns in `x`, [#4113](https://github.com/Rdatatable/data.table/issues/4113). Thanks to @MichaelChirico for the suggestion and @venom1204 for the PR. -14. `nafill()` and `setnafill()` gain a `limit` argument to restrict the maximum number of consecutive `NA` values filled during `locf` or `nocb` operations, [#7677](https://github.com/Rdatatable/data.table/issues/7677). Thanks to @jaynewton for the suggestion and @venom1204 for the PR. +14. `nafill()` and `setnafill()` gain a `limit` argument to restrict the maximum number of consecutive `NA` values filled, [#7677](https://github.com/Rdatatable/data.table/issues/7677). Thanks to @jaynewton for the suggestion and @venom1204 and @ben-schwen for the PR. ### BUG FIXES diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 3234d59422..6843e4aa89 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21995,8 +21995,10 @@ test(2388.05, setnafill(data.table(a=c(1, NA, NA)), type="locf", limit=1), data. test(2388.06, nafill(c(1, NA, NA), type="locf", limit=0), c(1, NA, NA)) test(2388.07, nafill(c(NA, 1), "locf", fill=0, limit=0), c(0, 1)) test(2388.08, nafill(c(1, NA, NA), type="locf", limit=1.5), c(1, 1, NA)) -test(2388.09, nafill(c(1, NA, NA), type="const", fill=0, limit=1), c(1, 0, 0)) +test(2388.09, nafill(c(1, NA, NA), type="const", fill=0, limit=1), c(1, 0, NA)) lims = list(-1, NA, NaN, NULL, c(1, 2), 1+2i); for (i in seq_along(lims)) { test(2388.09 + i / 100, nafill(1:5, limit=lims[[i]]), error="limit must be a non-negative scalar numeric") } test(2388.16, setnafill(data.table(a=1:5), limit=-1), error="limit must be a non-negative scalar numeric") test(2388.17, nafill(c(1.1, NA, NA, 4.4), type="nocb", limit=1), c(1.1, NA, 4.4, 4.4)) test(2388.18, nafill(c(1.1, NA, NA), type="locf", limit=1, nan=NaN), c(1.1, 1.1, NA)) +test(2388.19, nafill(c(NA, NA, 3, NA, NA, NA, 9), type="const", fill=0, limit=2), c(0, 0, 3, 0, 0, NA, 9)) +test(2388.20, nafill(c(1, NA, NA), type="const", fill=0, limit=0), c(1, NA, NA)) diff --git a/man/nafill.Rd b/man/nafill.Rd index c132a3a03c..ea8029e2dd 100644 --- a/man/nafill.Rd +++ b/man/nafill.Rd @@ -19,7 +19,7 @@ setnafill(x, type=c("const", "locf", "nocb"), fill=NA, nan=NA, cols=seq_along(x) \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, character or logical vector specifying columns to be updated. A logical vector must be the same length as the number of columns in \code{x}. } - \item{limit}{The maximum number of consecutive \code{NA} values to fill. Must be a non-negative scalar numeric. Default is \code{Inf}. Fractional values are truncated via \code{floor}. This argument is ignored when \code{type="const"}.} + \item{limit}{The maximum number of consecutive \code{NA} values to fill. Must be a non-negative scalar numeric. Default is \code{Inf}. Fractional values are truncated via \code{floor}.} } \details{ Supported types are \emph{logical}, \emph{integer}, \emph{double}, \emph{character}, and \emph{factor}, as well as classes built on top of these such as \code{Date}, \code{IDate}, and \code{POSIXct}. @@ -58,6 +58,7 @@ y = c(1, NA, NA, NA, 5) nafill(c(NA, 1), "locf", fill=0, limit=0) # Result: 0, 1 (Boundary case) nafill(y, "locf", limit=1) # Only fills the first NA nafill(y, "locf", limit=Inf) # Fills all NAs (default) +nafill(y, "const", fill=0, limit=1) } \seealso{ diff --git a/src/nafill.c b/src/nafill.c index 166254a2b6..9473e45fb7 100644 --- a/src/nafill.c +++ b/src/nafill.c @@ -5,13 +5,20 @@ void nafillDouble(double *x, uint_fast64_t nx, unsigned int type, double fill, b if (verbose) tic = omp_get_wtime(); if (type==0) { // const + uint_fast64_t fills = 0; if (nan_is_na) { for (uint_fast64_t i=0; idbl_v[i] = ISNAN(x[i]) ? fill : x[i]; + if (ISNAN(x[i])) { + if (fills < limit) { ans->dbl_v[i] = fill; fills++; } + else ans->dbl_v[i] = x[i]; + } else { ans->dbl_v[i] = x[i]; fills = 0; } } } else { for (uint_fast64_t i=0; idbl_v[i] = ISNA(x[i]) ? fill : x[i]; + if (ISNA(x[i])) { + if (fills < limit) { ans->dbl_v[i] = fill; fills++; } + else ans->dbl_v[i] = x[i]; + } else { ans->dbl_v[i] = x[i]; fills = 0; } } } } else if (type==1) { // locf @@ -65,8 +72,12 @@ void nafillInteger(int32_t *x, uint_fast64_t nx, unsigned int type, int32_t fill if (verbose) tic = omp_get_wtime(); if (type==0) { // const + uint_fast64_t fills = 0; for (uint_fast64_t i=0; iint_v[i] = x[i]==NA_INTEGER ? fill : x[i]; + if (x[i]==NA_INTEGER) { + if (fills < limit) { ans->int_v[i] = fill; fills++; } + else ans->int_v[i] = x[i]; + } else { ans->int_v[i] = x[i]; fills = 0; } } } else if (type==1) { // locf uint_fast64_t fills = 0; @@ -97,8 +108,12 @@ void nafillInteger64(int64_t *x, uint_fast64_t nx, unsigned int type, int64_t fi if (verbose) tic = omp_get_wtime(); if (type==0) { // const + uint_fast64_t fills = 0; for (uint_fast64_t i=0; iint64_v[i] = x[i]==NA_INTEGER64 ? fill : x[i]; + if (x[i]==NA_INTEGER64) { + if (fills < limit) { ans->int64_v[i] = fill; fills++; } + else ans->int64_v[i] = x[i]; + } else { ans->int64_v[i] = x[i]; fills = 0; } } } else if (type==1) { // locf uint_fast64_t fills = 0; @@ -129,9 +144,13 @@ void nafillString(const SEXP *x, uint_fast64_t nx, unsigned int type, SEXP fill, double tic=0.0; if (verbose) tic = omp_get_wtime(); - if (type==0) { // const 1Code has comments. Press enter to view. + if (type==0) { // const + uint_fast64_t fills = 0; for (uint_fast64_t i=0; ichar_v, i, x[i]==NA_STRING ? fill : x[i]); + if (x[i]==NA_STRING) { + if (fills < limit) { SET_STRING_ELT(ans->char_v, i, fill); fills++; } + else SET_STRING_ELT(ans->char_v, i, x[i]); + } else { SET_STRING_ELT(ans->char_v, i, x[i]); fills = 0; } } } else if (type==1) { // locf uint_fast64_t fills = 0; From cd015bf6d29da53bba7d77b6145ec587daca018b Mon Sep 17 00:00:00 2001 From: Benjamin Schwendinger Date: Thu, 20 Aug 2026 11:00:37 +0200 Subject: [PATCH 18/21] fix boundary case --- inst/tests/tests.Rraw | 2 +- man/nafill.Rd | 2 +- src/nafill.c | 40 ++++++++++++++++++++-------------------- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 6843e4aa89..6e84fbc303 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21993,7 +21993,7 @@ test(2388.03, nafill(c(NA, NA, 3, NA, NA), type="locf", limit=1), c(NA, NA, 3, 3 test(2388.04, nafill(c(1, NA, NA, 5), type="nocb", limit=1), c(1, NA, 5, 5)) test(2388.05, setnafill(data.table(a=c(1, NA, NA)), type="locf", limit=1), data.table(a=c(1, 1, NA))) test(2388.06, nafill(c(1, NA, NA), type="locf", limit=0), c(1, NA, NA)) -test(2388.07, nafill(c(NA, 1), "locf", fill=0, limit=0), c(0, 1)) +test(2388.07, nafill(c(NA, 1), "locf", fill=0, limit=0), c(NA, 1)) test(2388.08, nafill(c(1, NA, NA), type="locf", limit=1.5), c(1, 1, NA)) test(2388.09, nafill(c(1, NA, NA), type="const", fill=0, limit=1), c(1, 0, NA)) lims = list(-1, NA, NaN, NULL, c(1, 2), 1+2i); for (i in seq_along(lims)) { test(2388.09 + i / 100, nafill(1:5, limit=lims[[i]]), error="limit must be a non-negative scalar numeric") } diff --git a/man/nafill.Rd b/man/nafill.Rd index ea8029e2dd..ef61554db8 100644 --- a/man/nafill.Rd +++ b/man/nafill.Rd @@ -55,7 +55,7 @@ dt # limit= restricts the number of consecutive fills y = c(1, NA, NA, NA, 5) -nafill(c(NA, 1), "locf", fill=0, limit=0) # Result: 0, 1 (Boundary case) +nafill(c(NA, 1), "locf", fill=0, limit=0) # Result: NA, 1 (limit=0 fills nothing) nafill(y, "locf", limit=1) # Only fills the first NA nafill(y, "locf", limit=Inf) # Fills all NAs (default) nafill(y, "const", fill=0, limit=1) diff --git a/src/nafill.c b/src/nafill.c index 9473e45fb7..7fb76036b5 100644 --- a/src/nafill.c +++ b/src/nafill.c @@ -24,8 +24,8 @@ void nafillDouble(double *x, uint_fast64_t nx, unsigned int type, double fill, b } else if (type==1) { // locf uint_fast64_t fills = 0; if (nan_is_na) { - ans->dbl_v[0] = ISNAN(x[0]) ? fill : x[0]; - if (ISNAN(x[0])) fills = 1; + if (ISNAN(x[0]) && limit>0) { ans->dbl_v[0] = fill; fills = 1; } + else ans->dbl_v[0] = x[0]; for (uint_fast64_t i=1; idbl_v[i] = ans->dbl_v[i-1]; fills++; } @@ -33,8 +33,8 @@ void nafillDouble(double *x, uint_fast64_t nx, unsigned int type, double fill, b } else { ans->dbl_v[i] = x[i]; fills = 0; } } } else { - ans->dbl_v[0] = ISNA(x[0]) ? fill : x[0]; - if (ISNA(x[0])) fills = 1; + if (ISNA(x[0]) && limit>0) { ans->dbl_v[0] = fill; fills = 1; } + else ans->dbl_v[0] = x[0]; for (uint_fast64_t i=1; idbl_v[i] = ans->dbl_v[i-1]; fills++; } @@ -45,8 +45,8 @@ void nafillDouble(double *x, uint_fast64_t nx, unsigned int type, double fill, b } else if (type==2) { // nocb uint_fast64_t fills = 0; if (nan_is_na) { - ans->dbl_v[nx-1] = ISNAN(x[nx-1]) ? fill : x[nx-1]; - if (ISNAN(x[nx-1])) fills = 1; + if (ISNAN(x[nx-1]) && limit>0) { ans->dbl_v[nx-1] = fill; fills = 1; } + else ans->dbl_v[nx-1] = x[nx-1]; for (int_fast64_t i=nx-2; i>=0; i--) { if (ISNAN(x[i])) { if (fills < limit) { ans->dbl_v[i] = ans->dbl_v[i+1]; fills++; } @@ -54,8 +54,8 @@ void nafillDouble(double *x, uint_fast64_t nx, unsigned int type, double fill, b } else { ans->dbl_v[i] = x[i]; fills = 0; } } } else { - ans->dbl_v[nx-1] = ISNA(x[nx-1]) ? fill : x[nx-1]; - if (ISNA(x[nx-1])) fills = 1; + if (ISNA(x[nx-1]) && limit>0) { ans->dbl_v[nx-1] = fill; fills = 1; } + else ans->dbl_v[nx-1] = x[nx-1]; for (int_fast64_t i=nx-2; i>=0; i--) { if (ISNA(x[i])) { if (fills < limit) { ans->dbl_v[i] = ans->dbl_v[i+1]; fills++; } @@ -81,8 +81,8 @@ void nafillInteger(int32_t *x, uint_fast64_t nx, unsigned int type, int32_t fill } } else if (type==1) { // locf uint_fast64_t fills = 0; - ans->int_v[0] = x[0]==NA_INTEGER ? fill : x[0]; - if (x[0]==NA_INTEGER) fills = 1; + if (x[0]==NA_INTEGER && limit>0) { ans->int_v[0] = fill; fills = 1; } + else ans->int_v[0] = x[0]; for (uint_fast64_t i=1; iint_v[i] = ans->int_v[i-1]; fills++; } @@ -91,8 +91,8 @@ void nafillInteger(int32_t *x, uint_fast64_t nx, unsigned int type, int32_t fill } } else if (type==2) { // nocb uint_fast64_t fills = 0; - ans->int_v[nx-1] = x[nx-1]==NA_INTEGER ? fill : x[nx-1]; - if (x[nx-1]==NA_INTEGER) fills = 1; + if (x[nx-1]==NA_INTEGER && limit>0) { ans->int_v[nx-1] = fill; fills = 1; } + else ans->int_v[nx-1] = x[nx-1]; for (int_fast64_t i=nx-2; i>=0; i--) { if (x[i]==NA_INTEGER) { if (fills < limit) { ans->int_v[i] = ans->int_v[i+1]; fills++; } @@ -117,8 +117,8 @@ void nafillInteger64(int64_t *x, uint_fast64_t nx, unsigned int type, int64_t fi } } else if (type==1) { // locf uint_fast64_t fills = 0; - ans->int64_v[0] = x[0]==NA_INTEGER64 ? fill : x[0]; - if (x[0]==NA_INTEGER64) fills = 1; + if (x[0]==NA_INTEGER64 && limit>0) { ans->int64_v[0] = fill; fills = 1; } + else ans->int64_v[0] = x[0]; for (uint_fast64_t i=1; iint64_v[i] = ans->int64_v[i-1]; fills++; } @@ -127,8 +127,8 @@ void nafillInteger64(int64_t *x, uint_fast64_t nx, unsigned int type, int64_t fi } } else if (type==2) { // nocb uint_fast64_t fills = 0; - ans->int64_v[nx-1] = x[nx-1]==NA_INTEGER64 ? fill : x[nx-1]; - if (x[nx-1]==NA_INTEGER64) fills = 1; + if (x[nx-1]==NA_INTEGER64 && limit>0) { ans->int64_v[nx-1] = fill; fills = 1; } + else ans->int64_v[nx-1] = x[nx-1]; for (int_fast64_t i=nx-2; i>=0; i--) { if (x[i]==NA_INTEGER64) { if (fills < limit) { ans->int64_v[i] = ans->int64_v[i+1]; fills++; } @@ -154,8 +154,8 @@ void nafillString(const SEXP *x, uint_fast64_t nx, unsigned int type, SEXP fill, } } else if (type==1) { // locf uint_fast64_t fills = 0; - SET_STRING_ELT(ans->char_v, 0, x[0]==NA_STRING ? fill : x[0]); - if (x[0]==NA_STRING) fills = 1; + if (x[0]==NA_STRING && limit>0) { SET_STRING_ELT(ans->char_v, 0, fill); fills = 1; } + else SET_STRING_ELT(ans->char_v, 0, x[0]); const SEXP* thisans = SEXPPTR_RO(ans->char_v); // takes out STRING_ELT from loop for (uint_fast64_t i=1; ichar_v, nx-1, x[nx-1]==NA_STRING ? fill : x[nx-1]); - if (x[nx-1]==NA_STRING) fills = 1; + if (x[nx-1]==NA_STRING && limit>0) { SET_STRING_ELT(ans->char_v, nx-1, fill); fills = 1; } + else SET_STRING_ELT(ans->char_v, nx-1, x[nx-1]); const SEXP* thisans = SEXPPTR_RO(ans->char_v); // takes out STRING_ELT from loop for (int_fast64_t i=nx-2; i>=0; i--) { if (x[i]==NA_STRING) { From 12a9514d74c5e70061665b4443fac33647cd1aa8 Mon Sep 17 00:00:00 2001 From: Benjamin Schwendinger Date: Thu, 20 Aug 2026 11:42:29 +0200 Subject: [PATCH 19/21] update tests --- inst/tests/tests.Rraw | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 6e84fbc303..6d55daee39 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21987,18 +21987,20 @@ test(2387.05, setnafill(copy(DT3), type="locf", cols=c(TRUE,NA,FALSE)), error="' test(2387.06, setnafill(copy(DT3), type="locf", cols=c(TRUE,FALSE)), error="'cols' is a logical vector of length 2 but there are 3 columns") # #7677: add limit argument to nafill() -test(2388.01, list(nafill(c(1, NA, NA, NA, 5), type="locf", limit=1), nafill(c(1, NA, NA, 5, NA, NA, NA, 9), type="locf", limit=2), nafill(c(1, NA, NA, 4, NA), type="locf", limit=Inf)), list(c(1, 1, NA, NA, 5), c(1, 1, 1, 5, 5, 5, NA, 9), c(1, 1, 1, 4, 4))) -test(2388.02, list(nafill(c(1L, NA, NA), type="locf", limit=1), nafill(c("a", NA, NA), type="locf", limit=1)), list(c(1L, 1L, NA), c("a", "a", NA))) -test(2388.03, nafill(c(NA, NA, 3, NA, NA), type="locf", limit=1), c(NA, NA, 3, 3, NA)) -test(2388.04, nafill(c(1, NA, NA, 5), type="nocb", limit=1), c(1, NA, 5, 5)) -test(2388.05, setnafill(data.table(a=c(1, NA, NA)), type="locf", limit=1), data.table(a=c(1, 1, NA))) -test(2388.06, nafill(c(1, NA, NA), type="locf", limit=0), c(1, NA, NA)) -test(2388.07, nafill(c(NA, 1), "locf", fill=0, limit=0), c(NA, 1)) -test(2388.08, nafill(c(1, NA, NA), type="locf", limit=1.5), c(1, 1, NA)) -test(2388.09, nafill(c(1, NA, NA), type="const", fill=0, limit=1), c(1, 0, NA)) -lims = list(-1, NA, NaN, NULL, c(1, 2), 1+2i); for (i in seq_along(lims)) { test(2388.09 + i / 100, nafill(1:5, limit=lims[[i]]), error="limit must be a non-negative scalar numeric") } -test(2388.16, setnafill(data.table(a=1:5), limit=-1), error="limit must be a non-negative scalar numeric") -test(2388.17, nafill(c(1.1, NA, NA, 4.4), type="nocb", limit=1), c(1.1, NA, 4.4, 4.4)) -test(2388.18, nafill(c(1.1, NA, NA), type="locf", limit=1, nan=NaN), c(1.1, 1.1, NA)) -test(2388.19, nafill(c(NA, NA, 3, NA, NA, NA, 9), type="const", fill=0, limit=2), c(0, 0, 3, 0, 0, NA, 9)) -test(2388.20, nafill(c(1, NA, NA), type="const", fill=0, limit=0), c(1, NA, NA)) +test(2388.01, nafill(c(1, NA, NA, 5, NA, NA, NA, 9), type="locf", limit=2), c(1, 1, 1, 5, 5, 5, NA, 9)) +test(2388.02, nafill(c(1, NA, NA, 4, NA), type="locf", limit=Inf), c(1, 1, 1, 4, 4)) +test(2388.03, nafill(c(1L, NA, NA), type="locf", limit=1), c(1L, 1L, NA)) +test(2388.04, nafill(c("a", NA, NA), type="locf", limit=1), c("a", "a", NA)) +test(2388.05, nafill(c(NA, NA, 3, NA, NA), type="locf", limit=1), c(NA, NA, 3, 3, NA)) +test(2388.06, nafill(c(1, NA, NA, 5), type="nocb", limit=1), c(1, NA, 5, 5)) +test(2388.07, setnafill(data.table(a=c(1, NA, NA)), type="locf", limit=1), data.table(a=c(1, 1, NA))) +test(2388.08, nafill(c(1, NA, NA), type="locf", limit=0), c(1, NA, NA)) +test(2388.09, nafill(c(NA, 1), "locf", fill=0, limit=0), c(NA, 1)) +test(2388.10, nafill(c(1, NA, NA), type="locf", limit=1.5), c(1, 1, NA)) +test(2388.11, nafill(c(1, NA, NA), type="const", fill=0, limit=1), c(1, 0, NA)) +lims = list(-1, NA, NaN, NULL, c(1, 2), 1+2i); for (i in seq_along(lims)) { test(2388.12 + i / 1000, nafill(1:5, limit=lims[[i]]), error="limit must be a non-negative scalar numeric") } +test(2388.13, setnafill(data.table(a=1:5), limit=-1), error="limit must be a non-negative scalar numeric") +test(2388.14, nafill(c(1.1, NA, NA, 4.4), type="nocb", limit=1), c(1.1, NA, 4.4, 4.4)) +test(2388.15, nafill(c(1.1, NA, NA), type="locf", limit=1, nan=NaN), c(1.1, 1.1, NA)) +test(2388.16, nafill(c(NA, NA, 3, NA, NA, NA, 9), type="const", fill=0, limit=2), c(0, 0, 3, 0, 0, NA, 9)) +test(2388.17, nafill(c(1, NA, NA), type="const", fill=0, limit=0), c(1, NA, NA)) From e8bc1763522a0e5e34490510866edc4ff7f10083 Mon Sep 17 00:00:00 2001 From: Benjamin Schwendinger Date: Thu, 20 Aug 2026 11:58:58 +0200 Subject: [PATCH 20/21] pull var out of cases --- src/nafill.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/nafill.c b/src/nafill.c index 7fb76036b5..9c917567fd 100644 --- a/src/nafill.c +++ b/src/nafill.c @@ -4,8 +4,8 @@ void nafillDouble(double *x, uint_fast64_t nx, unsigned int type, double fill, b double tic=0.0; if (verbose) tic = omp_get_wtime(); + uint_fast64_t fills = 0; if (type==0) { // const - uint_fast64_t fills = 0; if (nan_is_na) { for (uint_fast64_t i=0; i0) { ans->dbl_v[0] = fill; fills = 1; } else ans->dbl_v[0] = x[0]; @@ -43,7 +42,6 @@ void nafillDouble(double *x, uint_fast64_t nx, unsigned int type, double fill, b } } } else if (type==2) { // nocb - uint_fast64_t fills = 0; if (nan_is_na) { if (ISNAN(x[nx-1]) && limit>0) { ans->dbl_v[nx-1] = fill; fills = 1; } else ans->dbl_v[nx-1] = x[nx-1]; @@ -71,8 +69,8 @@ void nafillInteger(int32_t *x, uint_fast64_t nx, unsigned int type, int32_t fill double tic=0.0; if (verbose) tic = omp_get_wtime(); + uint_fast64_t fills = 0; if (type==0) { // const - uint_fast64_t fills = 0; for (uint_fast64_t i=0; iint_v[i] = fill; fills++; } @@ -80,7 +78,6 @@ void nafillInteger(int32_t *x, uint_fast64_t nx, unsigned int type, int32_t fill } else { ans->int_v[i] = x[i]; fills = 0; } } } else if (type==1) { // locf - uint_fast64_t fills = 0; if (x[0]==NA_INTEGER && limit>0) { ans->int_v[0] = fill; fills = 1; } else ans->int_v[0] = x[0]; for (uint_fast64_t i=1; iint_v[i] = x[i]; fills = 0; } } } else if (type==2) { // nocb - uint_fast64_t fills = 0; if (x[nx-1]==NA_INTEGER && limit>0) { ans->int_v[nx-1] = fill; fills = 1; } else ans->int_v[nx-1] = x[nx-1]; for (int_fast64_t i=nx-2; i>=0; i--) { @@ -107,8 +103,8 @@ void nafillInteger64(int64_t *x, uint_fast64_t nx, unsigned int type, int64_t fi double tic=0.0; if (verbose) tic = omp_get_wtime(); + uint_fast64_t fills = 0; if (type==0) { // const - uint_fast64_t fills = 0; for (uint_fast64_t i=0; iint64_v[i] = fill; fills++; } @@ -116,7 +112,6 @@ void nafillInteger64(int64_t *x, uint_fast64_t nx, unsigned int type, int64_t fi } else { ans->int64_v[i] = x[i]; fills = 0; } } } else if (type==1) { // locf - uint_fast64_t fills = 0; if (x[0]==NA_INTEGER64 && limit>0) { ans->int64_v[0] = fill; fills = 1; } else ans->int64_v[0] = x[0]; for (uint_fast64_t i=1; iint64_v[i] = x[i]; fills = 0; } } } else if (type==2) { // nocb - uint_fast64_t fills = 0; if (x[nx-1]==NA_INTEGER64 && limit>0) { ans->int64_v[nx-1] = fill; fills = 1; } else ans->int64_v[nx-1] = x[nx-1]; for (int_fast64_t i=nx-2; i>=0; i--) { @@ -144,8 +138,8 @@ void nafillString(const SEXP *x, uint_fast64_t nx, unsigned int type, SEXP fill, double tic=0.0; if (verbose) tic = omp_get_wtime(); + uint_fast64_t fills = 0; if (type==0) { // const - uint_fast64_t fills = 0; for (uint_fast64_t i=0; ichar_v, i, fill); fills++; } @@ -153,7 +147,6 @@ void nafillString(const SEXP *x, uint_fast64_t nx, unsigned int type, SEXP fill, } else { SET_STRING_ELT(ans->char_v, i, x[i]); fills = 0; } } } else if (type==1) { // locf - uint_fast64_t fills = 0; if (x[0]==NA_STRING && limit>0) { SET_STRING_ELT(ans->char_v, 0, fill); fills = 1; } else SET_STRING_ELT(ans->char_v, 0, x[0]); const SEXP* thisans = SEXPPTR_RO(ans->char_v); // takes out STRING_ELT from loop @@ -164,7 +157,6 @@ void nafillString(const SEXP *x, uint_fast64_t nx, unsigned int type, SEXP fill, } else { SET_STRING_ELT(ans->char_v, i, x[i]); fills = 0; } } } else if (type==2) { // nocb - uint_fast64_t fills = 0; if (x[nx-1]==NA_STRING && limit>0) { SET_STRING_ELT(ans->char_v, nx-1, fill); fills = 1; } else SET_STRING_ELT(ans->char_v, nx-1, x[nx-1]); const SEXP* thisans = SEXPPTR_RO(ans->char_v); // takes out STRING_ELT from loop From 23386ac47c0a03c89d84fedb958de0bdabe648d5 Mon Sep 17 00:00:00 2001 From: Benjamin Schwendinger Date: Thu, 20 Aug 2026 12:37:06 +0200 Subject: [PATCH 21/21] add coverage --- inst/tests/nafill.Rraw | 3 +++ inst/tests/tests.Rraw | 13 ++++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/inst/tests/nafill.Rraw b/inst/tests/nafill.Rraw index 18b3605485..d603ebb4bb 100644 --- a/inst/tests/nafill.Rraw +++ b/inst/tests/nafill.Rraw @@ -249,6 +249,9 @@ if (test_bit64) { test(9.32, nafill(x, "nocb", 0), seti64(nafill(x, "nocb"), 5:6, as.integer64(0))) test(9.33, nafill(x, "locf", -1), seti64(nafill(x, "locf"), 1:2, as.integer64(-1))) test(9.34, nafill(x, "nocb", -1), seti64(nafill(x, "nocb"), 5:6, as.integer64(-1))) + test(9.35, nafill(x, "const", 0, limit=1), as.integer64(c(0,NA,3,4,0,NA))) + test(9.36, nafill(x, "locf", 0, limit=1), as.integer64(c(0,NA,3,4,4,NA))) + test(9.37, nafill(x, "nocb", 0, limit=1), as.integer64(c(NA,3,3,4,NA,0))) } # coerceAs verbose diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 6d55daee39..807916f45e 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21986,17 +21986,17 @@ test(2387.04, setnafill(copy(DT3), type="locf", cols=sapply(DT3, is.numeric)), d test(2387.05, setnafill(copy(DT3), type="locf", cols=c(TRUE,NA,FALSE)), error="'cols' contains NA at position 2") test(2387.06, setnafill(copy(DT3), type="locf", cols=c(TRUE,FALSE)), error="'cols' is a logical vector of length 2 but there are 3 columns") -# #7677: add limit argument to nafill() +# add limit argument to nafill() #7677 test(2388.01, nafill(c(1, NA, NA, 5, NA, NA, NA, 9), type="locf", limit=2), c(1, 1, 1, 5, 5, 5, NA, 9)) test(2388.02, nafill(c(1, NA, NA, 4, NA), type="locf", limit=Inf), c(1, 1, 1, 4, 4)) test(2388.03, nafill(c(1L, NA, NA), type="locf", limit=1), c(1L, 1L, NA)) test(2388.04, nafill(c("a", NA, NA), type="locf", limit=1), c("a", "a", NA)) -test(2388.05, nafill(c(NA, NA, 3, NA, NA), type="locf", limit=1), c(NA, NA, 3, 3, NA)) +test(2388.05, nafill(c(NA, NA, 3, NA, NA), type="const", limit=1, fill=3), c(3, NA, 3, 3, NA)) test(2388.06, nafill(c(1, NA, NA, 5), type="nocb", limit=1), c(1, NA, 5, 5)) test(2388.07, setnafill(data.table(a=c(1, NA, NA)), type="locf", limit=1), data.table(a=c(1, 1, NA))) test(2388.08, nafill(c(1, NA, NA), type="locf", limit=0), c(1, NA, NA)) test(2388.09, nafill(c(NA, 1), "locf", fill=0, limit=0), c(NA, 1)) -test(2388.10, nafill(c(1, NA, NA), type="locf", limit=1.5), c(1, 1, NA)) +test(2388.10, nafill(c(NA, 1, NA), type="nocb", limit=1.5), c(1, 1, NA)) test(2388.11, nafill(c(1, NA, NA), type="const", fill=0, limit=1), c(1, 0, NA)) lims = list(-1, NA, NaN, NULL, c(1, 2), 1+2i); for (i in seq_along(lims)) { test(2388.12 + i / 1000, nafill(1:5, limit=lims[[i]]), error="limit must be a non-negative scalar numeric") } test(2388.13, setnafill(data.table(a=1:5), limit=-1), error="limit must be a non-negative scalar numeric") @@ -22004,3 +22004,10 @@ test(2388.14, nafill(c(1.1, NA, NA, 4.4), type="nocb", limit=1), c(1.1, NA, 4.4, test(2388.15, nafill(c(1.1, NA, NA), type="locf", limit=1, nan=NaN), c(1.1, 1.1, NA)) test(2388.16, nafill(c(NA, NA, 3, NA, NA, NA, 9), type="const", fill=0, limit=2), c(0, 0, 3, 0, 0, NA, 9)) test(2388.17, nafill(c(1, NA, NA), type="const", fill=0, limit=0), c(1, NA, NA)) +test(2388.18, nafill(c(NA_integer_, NA_integer_), type="const", fill=1L, limit=1), c(1L, NA)) +test(2388.19, nafill(c(NA, 1L, NA), type="nocb", limit=1), c(1L, 1L, NA)) +test(2388.20, nafill(c(1, NA, NA), type="const", fill=0, limit=1, nan=NaN), c(1, 0, NA)) +test(2388.21, nafill(c(1, NA, NA, 5), type="nocb", limit=1, nan=NaN), c(1, NA, 5, 5)) +test(2388.22, nafill(c(1L, NA, NA, 5L), type="nocb", limit=1), c(1L, NA, 5L, 5L)) +test(2388.23, nafill(c("a", NA, NA), type="const", fill="x", limit=1), c("a", "x", NA)) +test(2388.24, nafill(c("a", NA, NA, "z"), type="nocb", limit=1), c("a", NA, "z", "z"))