Skip to content

Add limit parameter to nafill and setnafill #7677 #7819

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
venom1204 wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add limit parameter to nafill and setnafill #7677 #7819

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dd875ac
updated changes
venom1204 Jul 18, 2026
998fa2a
Merge remote-tracking branch 'origin/master' into issue777666777
venom1204 Jul 25, 2026
5bbc47d
added changes
venom1204 Jul 25, 2026
14af4f4
..
venom1204 Jul 25, 2026
7c59afe
..
venom1204 Jul 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
14 changes: 10 additions & 4 deletions R/shift.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +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) {
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 || 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)))
}

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 || 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))))
}
24 changes: 24 additions & 0 deletions inst/tests/tests.Rraw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21850,3 +21850,27 @@ a = c(a=1L)
test(2379.12, as.IDate(d) + 1L, as.IDate(as.Date(d) + 1L))
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(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), 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, 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))
11 changes: 9 additions & 2 deletions man/nafill.Rd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,16 @@
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. }
\item{type}{ Character, one of \emph{"const"}, \emph{"locf"} or \emph{"nocb"}. Defaults to \code{"const"}. }
\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}.
Expand Down Expand Up @@ -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=Inf) # Fills all NAs (default)

}
\seealso{
\code{\link{shift}}, \code{\link{data.table}}, \code{\link{fcoalesce}}
Expand Down
8 changes: 5 additions & 3 deletions src/data.table.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
88 changes: 69 additions & 19 deletions src/nafill.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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();
Expand All @@ -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;
Comment on lines 20 to +21

@ben-schwen ben-schwen Jul 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not need this double branching on ISNAN(x[0]). This will also happen at the other cases below!

for (uint_fast64_t i=1; i<nx; 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[0] = ISNA(x[0]) ? fill : x[0];
if (ISNA(x[0])) fills = 1;
for (uint_fast64_t i=1; i<nx; 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; }
}
}
} 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();
Expand All @@ -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; i<nx; 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; }
}
} 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();
Expand All @@ -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; i<nx; 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; }
}
} 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();
Expand All @@ -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; i<nx; 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; }
}
} 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)
Expand All @@ -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();

Expand All @@ -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];

@ben-schwen ben-schwen Jul 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be cleaner to represent as uint64_fast64_t since we also compare with it later

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
Expand Down Expand Up @@ -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;
}
}
Expand Down
Loading