From 01bb0904ead24f8d054451e55de8a807978e245e Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Wed, 20 Nov 2019 06:38:29 +0800 Subject: [PATCH 1/2] j auto-naming might delete an explicit NULL --- NEWS.md | 2 +- R/data.table.R | 3 ++- inst/tests/tests.Rraw | 2 ++ 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index 0b346019b0..d03a54eb1a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -6,7 +6,7 @@ ## NEW FEATURES -1. `DT[, {...; .(A,B)}]` (when `.()` is the final item of a multi-statement `{...}`) now auto-names the columns `A` and `B` (just like `DT[, .(A,B)]`) rather than `V1` and `V2`, [#2478](https://github.com/Rdatatable/data.table/issues/2478) [#609](https://github.com/Rdatatable/data.table/issues/609). Similarly, `DT[, if (.N>1) .(B), by=A]` now auto-names the column `B` rather than `V1`. Explicit names are unaffected; e.g. `DT[, {... y= ...; .(A=C+y)}, by=...]` named the column `A` before, and still does. +1. `DT[, {...; .(A,B)}]` (when `.()` is the final item of a multi-statement `{...}`) now auto-names the columns `A` and `B` (just like `DT[, .(A,B)]`) rather than `V1` and `V2`, [#2478](https://github.com/Rdatatable/data.table/issues/2478) [#609](https://github.com/Rdatatable/data.table/issues/609). Similarly, `DT[, if (.N>1) .(B), by=A]` now auto-names the column `B` rather than `V1`. Explicit names are unaffected; e.g. `DT[, {... y= ...; .(A=C+y)}, by=...]` named the column `A` before, and still does. Thanks also to @renkun-ken for catching a mistake here before release, [#4061](https://github.com/Rdatatable/data.table/issues/4061). ## BUG FIXES diff --git a/R/data.table.R b/R/data.table.R index f8de1462c0..2c6ee379ab 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -886,7 +886,8 @@ replace_dot_alias = function(e) { return(q) # else empty list is needed for test 468: adding an empty list column } if (q[[1L]] == '{') { - q[[length(q)]] = do_j_names(q[[length(q)]]) + qlen = length(q) + if (!is.null(q[[qlen]])) q[[qlen]] = do_j_names(q[[qlen]]) return(q) } if (q[[1L]] == 'if') { diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 157e68e8e7..f10fe26ce1 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -16378,6 +16378,8 @@ test(2121.3, DT[ , if (.N > 1L) .(b), by=a], DT[1:2]) test(2121.4, DT[ , if (.N > 1L) .(b) else .(c=b), by=a], DT[ , .(a, c=b)], warning="Different branches of j expression produced different auto-named columns") test(2121.5, DT[, .(.N=.N), by=a], data.table(a=c(1,2), .N=2:1)) # user supplied names preside over autoname dropping leading dot +## { ending in NULL should retain that NULL, #4061 +test(2121.6, DT[ , {.(a, b=b+1); NULL}], NULL) ################################### From f9c360ef97690dc033f71cf891ccc680af6310af Mon Sep 17 00:00:00 2001 From: mattdowle Date: Tue, 19 Nov 2019 19:24:12 -0800 Subject: [PATCH 2/2] news item; added 'i.e.' and referred very briefly to the issue Kun found --- NEWS.md | 2 +- R/data.table.R | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/NEWS.md b/NEWS.md index d03a54eb1a..a6edf8f3b5 100644 --- a/NEWS.md +++ b/NEWS.md @@ -6,7 +6,7 @@ ## NEW FEATURES -1. `DT[, {...; .(A,B)}]` (when `.()` is the final item of a multi-statement `{...}`) now auto-names the columns `A` and `B` (just like `DT[, .(A,B)]`) rather than `V1` and `V2`, [#2478](https://github.com/Rdatatable/data.table/issues/2478) [#609](https://github.com/Rdatatable/data.table/issues/609). Similarly, `DT[, if (.N>1) .(B), by=A]` now auto-names the column `B` rather than `V1`. Explicit names are unaffected; e.g. `DT[, {... y= ...; .(A=C+y)}, by=...]` named the column `A` before, and still does. Thanks also to @renkun-ken for catching a mistake here before release, [#4061](https://github.com/Rdatatable/data.table/issues/4061). +1. `DT[, {...; .(A,B)}]` (i.e. when `.()` is the final item of a multi-statement `{...}`) now auto-names the columns `A` and `B` (just like `DT[, .(A,B)]`) rather than `V1` and `V2`, [#2478](https://github.com/Rdatatable/data.table/issues/2478) [#609](https://github.com/Rdatatable/data.table/issues/609). Similarly, `DT[, if (.N>1) .(B), by=A]` now auto-names the column `B` rather than `V1`. Explicit names are unaffected; e.g. `DT[, {... y= ...; .(A=C+y)}, by=...]` named the column `A` before, and still does. Thanks also to @renkun-ken for his go-first strong testing which caught an issue not caught by the test suite or by revdep testing, related to NULL being the last item, [#4061](https://github.com/Rdatatable/data.table/issues/4061). ## BUG FIXES diff --git a/R/data.table.R b/R/data.table.R index 2c6ee379ab..780e09029a 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -886,8 +886,7 @@ replace_dot_alias = function(e) { return(q) # else empty list is needed for test 468: adding an empty list column } if (q[[1L]] == '{') { - qlen = length(q) - if (!is.null(q[[qlen]])) q[[qlen]] = do_j_names(q[[qlen]]) + if (!is.null(q[[qlen<-length(q)]])) q[[qlen]] = do_j_names(q[[qlen]]) return(q) } if (q[[1L]] == 'if') {