diff --git a/NEWS.md b/NEWS.md index 6fa4e801fb..38002f3dd8 100644 --- a/NEWS.md +++ b/NEWS.md @@ -174,6 +174,8 @@ 33. `fread(text="a,b,c")` (where input data contains no `\n` but `text=` has been used) now works instead of error `file not found: a,b,c`, [#4689](https://github.com/Rdatatable/data.table/issues/4689). Thanks to @trainormg for reporting, and @ben-schwen for the PR. +34. `na.omit(DT)` did not remove `NA` in `nanotime` columns, [#4744](https://github.com/Rdatatable/data.table/issues/4744). Thanks Jean-Mathieu Vermosen for reporting, and Michael Chirico for the PR. + ## NOTES 1. New feature 29 in v1.12.4 (Oct 2019) introduced zero-copy coercion. Our thinking is that requiring you to get the type right in the case of `0` (type double) vs `0L` (type integer) is too inconvenient for you the user. So such coercions happen in `data.table` automatically without warning. Thanks to zero-copy coercion there is no speed penalty, even when calling `set()` many times in a loop, so there's no speed penalty to warn you about either. However, we believe that assigning a character value such as `"2"` into an integer column is more likely to be a user mistake that you would like to be warned about. The type difference (character vs integer) may be the only clue that you have selected the wrong column, or typed the wrong variable to be assigned to that column. For this reason we view character to numeric-like coercion differently and will warn about it. If it is correct, then the warning is intended to nudge you to wrap the RHS with `as.()` so that it is clear to readers of your code that a coercion from character to that type is intended. For example : diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index ec9cc1e232..b362096ff9 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -17912,3 +17912,8 @@ test(2203.20, tstrsplit(w, "/", type.convert=list()), error="not support empty l test(2204, as.data.table(mtcars, keep.rownames='model', key='model'), setnames(setkey(as.data.table(mtcars, keep.rownames = TRUE), rn), 'rn', 'model')) +# na.omit works for nanotime, #4744 +if (test_nanotime) { + DT = data.table(time=nanotime(c(1,NA,3))) + test(2205, na.omit(DT), DT[c(1,3)]) +} diff --git a/src/assign.c b/src/assign.c index 0dc38c9b0a..c84ca276a5 100644 --- a/src/assign.c +++ b/src/assign.c @@ -710,8 +710,8 @@ const char *memrecycle(const SEXP target, const SEXP where, const int start, con snprintf(targetDesc, 500, colnum==0 ? _("target vector") : _("column %d named '%s'"), colnum, colname); int protecti=0; const bool sourceIsFactor=isFactor(source), targetIsFactor=isFactor(target); - const bool sourceIsI64=isReal(source) && Rinherits(source, char_integer64); - const bool targetIsI64=isReal(target) && Rinherits(target, char_integer64); + const bool sourceIsI64=isReal(source) && INHERITS(source, char_integer64); + const bool targetIsI64=isReal(target) && INHERITS(target, char_integer64); if (sourceIsFactor || targetIsFactor) { if (!targetIsFactor) { if (!isString(target) && !isNewList(target)) @@ -1116,7 +1116,7 @@ void writeNA(SEXP v, const int from, const int n, const bool listNA) for (int i=from; i<=to; ++i) vd[i] = NA_INTEGER; } break; case REALSXP: { - if (Rinherits(v, char_integer64)) { // Rinherits covers nanotime too which inherits from integer64 via S4 extends + if (INHERITS(v, char_integer64)) { int64_t *vd = (int64_t *)REAL(v); for (int i=from; i<=to; ++i) vd[i] = NA_INTEGER64; } else { diff --git a/src/between.c b/src/between.c index c5d91b30c0..899ea1d94e 100644 --- a/src/between.c +++ b/src/between.c @@ -83,8 +83,8 @@ SEXP between(SEXP x, SEXP lower, SEXP upper, SEXP incbounds, SEXP NAboundsArg, S } break; case REALSXP: - if (Rinherits(x, char_integer64)) { - if (!Rinherits(lower, char_integer64) || !Rinherits(upper, char_integer64)) + if (INHERITS(x, char_integer64)) { + if (!INHERITS(lower, char_integer64) || !INHERITS(upper, char_integer64)) error(_("x is integer64 but lower and/or upper are not.")); // e.g. between(int64, character, character) const int64_t *lp = (int64_t *)REAL(lower); const int64_t *up = (int64_t *)REAL(upper); @@ -111,7 +111,7 @@ SEXP between(SEXP x, SEXP lower, SEXP upper, SEXP incbounds, SEXP NAboundsArg, S } if (verbose) Rprintf(_("between parallel processing of integer64 took %8.3fs\n"), omp_get_wtime()-tic); } else { - if (Rinherits(lower, char_integer64) || Rinherits(upper, char_integer64)) + if (INHERITS(lower, char_integer64) || INHERITS(upper, char_integer64)) error(_("x is not integer64 but lower and/or upper is integer64. Please align classes.")); const double *lp = REAL(lower); const double *up = REAL(upper); diff --git a/src/coalesce.c b/src/coalesce.c index 558d2d4da5..75ce2e2e00 100644 --- a/src/coalesce.c +++ b/src/coalesce.c @@ -74,7 +74,7 @@ SEXP coalesce(SEXP x, SEXP inplaceArg) { } } break; case REALSXP: { - if (Rinherits(first, char_integer64)) { // Rinherits() is true for nanotime + if (INHERITS(first, char_integer64)) { int64_t *xP=(int64_t *)REAL(first), finalVal=NA_INTEGER64; int k=0; for (int j=0; j