diff --git a/NEWS.md b/NEWS.md index 465b3505df..6a218bb6ba 100644 --- a/NEWS.md +++ b/NEWS.md @@ -135,6 +135,9 @@ 20. `uniqueN(DT, by=character())` is now equivalent to `uniqueN(DT)` rather than internal error `'by' is either not integer or is length 0`, [#4594](https://github.com/Rdatatable/data.table/issues/4594). Thanks Marco Colombo for the report, and Michael Chirico for the PR. Similarly for `unique()`, `duplicated()` and `anyDuplicated()`. +21. `melt()` on a `data.table` with `list` columns for `measure.vars` would silently ignore `na.rm=TRUE`, [#5044](https://github.com/Rdatatable/data.table/issues/5044). Now the same logic as `is.na()` from base R is used; i.e. if list element is scalar NA then it is considered missing and removed. Thanks to Toby Dylan Hocking for the PRs. + + ## 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 a0e92fc729..c1a6b1ca34 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -3060,6 +3060,13 @@ test(1034, as.data.table(x<-as.character(sample(letters, 5))), data.table(V1=x)) # na.rm=TRUE with list column value, PR#4737 test(1035.016, melt(data.table(a1=1, b1=list(1:2), b2=list(c('foo','bar'))), na.rm=TRUE, measure.vars=list(a="a1", b=c("b1","b2"))), data.table(variable=factor(1), a=1, b=list(1:2))) test(1035.017, melt(data.table(a1=1, b1=1, b2=2), na.rm=TRUE, measure.vars=list(a="a1", b=c("b1","b2"))), data.table(variable=factor(1), a=1, b=1))#this worked even before the PR. + DT.list.missing = data.table(l1=list(1,NA), l2=list(NA,2), n34=c(3,4), NA5=c(NA,5)) + test(1035.0180, melt(DT.list.missing, measure.vars=c("n34","NA5"), na.rm=TRUE)[["value"]], c(3,4,5)) + test(1035.0181, melt(DT.list.missing, measure.vars=c("l1","l2"), na.rm=TRUE)[["value"]], list(1,2)) + test(1035.0182, melt(DT.list.missing, measure.vars=c("l1","n34"), na.rm=TRUE)[["value"]], list(1,3,4), warning="are not all of the same type") + test(1035.0183, melt(DT.list.missing, measure.vars=c("l1","NA5"), na.rm=TRUE)[["value"]], list(1,5), warning="are not all of the same type") + test(1035.0184, melt(DT.list.missing, measure.vars=list(l=c("l1","l2"), n=c("n34","NA5")), na.rm=TRUE), data.table(variable=factor(1:2), l=list(1,2), n=c(3,5))) + test(1035.0185, melt(data.table(l=list(c(NA,NA), NA, NA_integer_, NA_real_, NA_complex_, NA_character_, if(test_bit64)NA_integer64_)), measure.vars="l", na.rm=TRUE)[["value"]], list(c(NA,NA))) ans1 = cbind(DT[, c(1,2,8), with=FALSE], variable=factor("l_1")) ans1[, value := DT$l_1] @@ -6514,7 +6521,7 @@ test(1459.12, .Call("CsubsetDT", DT, 5L, seq_along(DT)), setDT(as.data.frame(DT) # Test for na.omit with list, raw and complex types DT = data.table(x=c(1L,1L,NA), y=c(NA, NA, 1), z=as.raw(1:3), w=list(1,NA,2), v=c(1+5i, NA, NA)) -test(1460.1, na.omit(DT, cols="w"), DT) +test(1460.1, na.omit(DT, cols="w"), DT[c(1,3)]) test(1460.2, na.omit(DT, cols="v"), DT[1]) test(1460.3, na.omit(DT, cols=c("v", "y")), DT[0]) test(1460.4, na.omit(DT, cols=c("z", "v")), DT[1]) diff --git a/man/melt.data.table.Rd b/man/melt.data.table.Rd index ddca733fe8..b31017356b 100644 --- a/man/melt.data.table.Rd +++ b/man/melt.data.table.Rd @@ -141,8 +141,11 @@ melt(DT, id.vars=1:2, measure.vars=patterns(f="^f_", d="^d_"), value.factor=TRUE # na.rm=TRUE removes rows with NAs in any 'value' columns melt(DT, id.vars=1:2, measure.vars=patterns("f_", "d_"), value.factor=TRUE, na.rm=TRUE) -# return 'NA' for missing columns, 'na.rm=TRUE' ignored due to list column -melt(DT, id.vars=1:2, measure.vars=patterns("l_", "c_"), na.rm=TRUE) +# 'na.rm=TRUE' also works with list column, but note that is.na only +# returns TRUE if the list element is a length=1 vector with an NA. +is.na(list(one.NA=NA, two.NA=c(NA,NA))) +melt(DT, id.vars=1:2, measure.vars=patterns("l_", "d_"), na.rm=FALSE) +melt(DT, id.vars=1:2, measure.vars=patterns("l_", "d_"), na.rm=TRUE) # measure list with missing/short entries results in output with runs of NA DT.missing.cols <- DT[, .(d_1, d_2, c_1, f_2)] diff --git a/src/fmelt.c b/src/fmelt.c index 8c204cb5ce..b33d99adb4 100644 --- a/src/fmelt.c +++ b/src/fmelt.c @@ -520,6 +520,7 @@ SEXP getvaluecols(SEXP DT, SEXP dtnames, Rboolean valfactor, Rboolean verbose, s for (int k=0; knrow; ++k) SET_STRING_ELT(target, j*data->nrow + k, STRING_ELT(thiscol, k)); } break; + //TODO complex value type: case CPLXSXP: { } break; case REALSXP : { double *dtarget = REAL(target); const double *dthiscol = REAL(thiscol); @@ -729,10 +730,21 @@ SEXP getidcols(SEXP DT, SEXP dtnames, Rboolean verbose, struct processData *data } break; case VECSXP : { - for (int j=0; jlmax; ++j) { - for (int k=0; knrow; ++k) { - SET_VECTOR_ELT(target, j*data->nrow + k, VECTOR_ELT(thiscol, k)); + if (data->narm) { + for (int j=0; jlmax; ++j) { + SEXP thisidx = VECTOR_ELT(data->naidx, j); + const int *ithisidx = INTEGER(thisidx); + const int thislen = length(thisidx); + for (int k=0; klmax; ++j) { + for (int k=0; knrow; ++k) { + SET_VECTOR_ELT(target, j*data->nrow + k, VECTOR_ELT(thiscol, k)); + } + } } } break; diff --git a/src/frank.c b/src/frank.c index 810baf85c5..2e9e14bcf1 100644 --- a/src/frank.c +++ b/src/frank.c @@ -19,7 +19,7 @@ SEXP dt_na(SEXP x, SEXP cols) { for (int i=0; i