From ca196b051903d42928e70413ca4cf291bb79c8ca Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Fri, 6 Sep 2019 09:41:25 +0800 Subject: [PATCH 1/2] Return full table for !all(FALSE) with subset --- NEWS.md | 2 ++ R/data.table.R | 5 +++-- inst/tests/tests.Rraw | 2 ++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index e3820812ee..c45595fc7f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -284,6 +284,8 @@ 38. `rbindlist` now returns correct idcols for lists with different length vectors, [#3785](https://github.com/Rdatatable/data.table/issues/3785), [#3786](https://github.com/Rdatatable/data.table/pull/3786). Thanks to @shrektan for the report and fix. +39. `DT[ , !rep(FALSE, ncol(DT)), with=FALSE]` correctly returns the full table, [#3013](https://github.com/Rdatatable/data.table/issues/3013). Thanks @alexnss for the report. + #### NOTES 1. `rbindlist`'s `use.names="check"` now emits its message for automatic column names (`"V[0-9]+"`) too, [#3484](https://github.com/Rdatatable/data.table/pull/3484). See news item 5 of v1.12.2 below. diff --git a/R/data.table.R b/R/data.table.R index 0d4418ec70..a94c26c3f6 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -645,7 +645,7 @@ replace_dot_alias = function(e) { j = eval(jsub, lapply(substring(..syms,3L), get, pos=parent.frame()), parent.frame()) } if (is.logical(j)) j <- which(j) - if (!length(j)) return( null.data.table() ) + if (!length(j) && !notj) return( null.data.table() ) if (is.factor(j)) j = as.character(j) # fix for FR: #4867 if (is.character(j)) { if (notj) { @@ -675,7 +675,8 @@ replace_dot_alias = function(e) { if (any(j>0L)) stop("j mixes positives and negatives") j = seq_along(x)[j] # all j are <0 here } - if (notj && length(j)) j = seq_along(x)[-j] + # 3013 -- handle !FALSE in column subset in j via logical+with + if (notj) j = seq_along(x)[if (length(j)) -j else TRUE] if (!length(j)) return(null.data.table()) return(.Call(CsubsetDT, x, irows, j)) } else { diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index da5e93ec3c..12b55b8ae5 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -15925,6 +15925,8 @@ test(2100.14, fifelse(c(T,F,NA),c(1,1,1),c(2,2,2),NA), c(1,2,NA)) DT = data.table(id=1:3, v=4:6, key="id") test(2101, DT[.(logical())], data.table(id=logical(), v=integer(), key="id")) +# DT[ , !rep(FALSE, ncol(DT)), with=FALSE] would exit before doing !, #3013 +test(2102, DT[ , !rep(FALSE, 2L), with=FALSE], DT) ################################### # Add new tests above this line # From 8e3a879606775d6c729f1cce17b1543356fa68ee Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Sun, 8 Sep 2019 20:17:37 +0800 Subject: [PATCH 2/2] add NEWS item for duplicate issue --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index c45595fc7f..45844eb2aa 100644 --- a/NEWS.md +++ b/NEWS.md @@ -284,7 +284,7 @@ 38. `rbindlist` now returns correct idcols for lists with different length vectors, [#3785](https://github.com/Rdatatable/data.table/issues/3785), [#3786](https://github.com/Rdatatable/data.table/pull/3786). Thanks to @shrektan for the report and fix. -39. `DT[ , !rep(FALSE, ncol(DT)), with=FALSE]` correctly returns the full table, [#3013](https://github.com/Rdatatable/data.table/issues/3013). Thanks @alexnss for the report. +39. `DT[ , !rep(FALSE, ncol(DT)), with=FALSE]` correctly returns the full table, [#3013](https://github.com/Rdatatable/data.table/issues/3013) and [#2917](https://github.com/Rdatatable/data.table/issues/2917). Thanks @alexnss and @DavidArenburg for the reports. #### NOTES