Skip to content

Commit

Permalink
Reset gap counter for each column
Browse files Browse the repository at this point in the history
Calling na.locf() on a multi-column xts object that is all NA except
for the first row did not carry the first row value forward. This was
caused by maxgap = nrow(x) and the gap counter not being reset after
looping over the first column. This would have also been a problem if
all but the last row were NA and na.locf() was called with fromLast =
FALSE.

Fixes #232.
  • Loading branch information
joshuaulrich committed Apr 14, 2018
1 parent af7464b commit 31b5911
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 0 deletions.
30 changes: 30 additions & 0 deletions inst/unitTests/runit.na.locf.R
Original file line number Diff line number Diff line change
Expand Up @@ -158,3 +158,33 @@ test.nalocf_by_column_xout <- function() {
checkEquals(x, as.xts(z), check.attributes = TRUE)
}
}

test.nalocf_by_column_1NA <- function() {
narow <- 1L
for (m in MODES) {
xdrow <- XDAT2[narow,]
xdat <- XDAT2 * NA
xdat[narow,] <- xdrow
storage.mode(xdat) <- m
zdat <- as.zoo(xdat)

x <- na.locf(xdat)
z <- na.locf(zdat)
checkEquals(x, as.xts(z), check.attributes = TRUE)
}
}

test.nalocf_by_column_1NA_fromLast <- function() {
narow <- nrow(XDAT2)
for (m in MODES) {
xdrow <- XDAT2[narow,]
xdat <- XDAT2 * NA
xdat[narow,] <- xdrow
storage.mode(xdat) <- m
zdat <- as.zoo(xdat)

x <- na.locf(xdat, fromLast = TRUE)
z <- na.locf(zdat, fromLast = TRUE)
checkEquals(x, as.xts(z), check.attributes = TRUE)
}
}
8 changes: 8 additions & 0 deletions src/na.c
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ SEXP na_locf (SEXP x, SEXP fromLast, SEXP _maxgap, SEXP _limit)
int_result[i] = int_x[i];
}
/* result[_first] now has first value fromLast=FALSE */
gap = 0;
for(i=_first+1; i<nr+j*nr; i++) {
int_result[i] = int_x[i];
if(int_result[i] == NA_LOGICAL && gap < maxgap) {
Expand All @@ -192,6 +193,7 @@ SEXP na_locf (SEXP x, SEXP fromLast, SEXP _maxgap, SEXP _limit)
/* nr-2 is first position to fill fromLast=TRUE */
for(j=0; j < nc; j++) {
int_result[nr-1+j*nr] = int_x[nr-1+j*nr];
gap = 0;
for(i=nr-2 + j*nr; i>=0+j*nr; i--) {
int_result[i] = int_x[i];
if(int_result[i] == NA_LOGICAL && gap < maxgap) {
Expand All @@ -213,6 +215,7 @@ SEXP na_locf (SEXP x, SEXP fromLast, SEXP _maxgap, SEXP _limit)
int_result[i] = int_x[i];
}
/* result[_first] now has first value fromLast=FALSE */
gap = 0;
for(i=_first+1; i<nr+j*nr; i++) {
int_result[i] = int_x[i];
if(int_result[i] == NA_INTEGER) {
Expand All @@ -238,6 +241,7 @@ SEXP na_locf (SEXP x, SEXP fromLast, SEXP _maxgap, SEXP _limit)
/* nr-2 is first position to fill fromLast=TRUE */
for(j=0; j < nc; j++) {
int_result[nr-1+j*nr] = int_x[nr-1+j*nr];
gap = 0;
for(i=nr-2 + j*nr; i>=0+j*nr; i--) {
int_result[i] = int_x[i];
if(int_result[i] == NA_INTEGER) {
Expand Down Expand Up @@ -272,6 +276,7 @@ SEXP na_locf (SEXP x, SEXP fromLast, SEXP _maxgap, SEXP _limit)
real_result[i] = real_x[i];
}
/* result[_first] now has first value fromLast=FALSE */
gap = 0;
for(i=_first+1; i<nr+j*nr; i++) {
real_result[i] = real_x[i];
if( ISNA(real_result[i]) || ISNAN(real_result[i])) {
Expand All @@ -296,6 +301,7 @@ SEXP na_locf (SEXP x, SEXP fromLast, SEXP _maxgap, SEXP _limit)
} else { /* fromLast=TRUE */
for(j=0; j < nc; j++) {
real_result[nr-1+j*nr] = real_x[nr-1+j*nr];
gap = 0;
for(i=nr-2 + j*nr; i>=0+j*nr; i--) {
real_result[i] = real_x[i];
if(ISNA(real_result[i]) || ISNAN(real_result[i])) {
Expand Down Expand Up @@ -328,6 +334,7 @@ SEXP na_locf (SEXP x, SEXP fromLast, SEXP _maxgap, SEXP _limit)
SET_STRING_ELT(result, i, STRING_ELT(x, i));
}
/* result[_first] now has first value fromLast=FALSE */
gap = 0;
for(i=_first+1; i<nr+j*nr; i++) {
SET_STRING_ELT(result, i, STRING_ELT(x, i));
if(STRING_ELT(x, i) == NA_STRING) {
Expand All @@ -352,6 +359,7 @@ SEXP na_locf (SEXP x, SEXP fromLast, SEXP _maxgap, SEXP _limit)
} else { /* fromLast=TRUE */
for(j=0; j < nc; j++) {
SET_STRING_ELT(result, nr-1+j*nr, STRING_ELT(x, nr-1+j*nr));
gap = 0;
for(i=nr-2 + j*nr; i>=0+j*nr; i--) {
SET_STRING_ELT(result, i, STRING_ELT(x, i));
if(STRING_ELT(result, i) == NA_STRING) {
Expand Down

0 comments on commit 31b5911

Please sign in to comment.