From 52f3bd511619c799037b6436ec07dfca7a18cc7b Mon Sep 17 00:00:00 2001 From: Cole Miller <57992489+ColeMiller1@users.noreply.github.com> Date: Thu, 16 Jul 2020 23:00:31 -0400 Subject: [PATCH 01/14] Update assign.c --- src/assign.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/assign.c b/src/assign.c index 88fc260655..c9f83ac638 100644 --- a/src/assign.c +++ b/src/assign.c @@ -318,7 +318,8 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values) error(_("data.table is NULL; malformed. A null data.table should be an empty list. typeof() should always return 'list' for data.table.")); // # nocov // Not possible to test because R won't permit attributes be attached to NULL (which is good and we like); warning from R 3.4.0+ tested by 944.5 } - const int nrow = LENGTH(dt) ? length(VECTOR_ELT(dt,0)) : + const bool is_null_dt = LENGTH(dt) == 0; // fix for 4597; null.data.table needs row names + const int nrow = !is_null_dt ? length(VECTOR_ELT(dt,0)) : (isNewList(values) && length(values) ? length(VECTOR_ELT(values,0)) : length(values)); // ^ when null data.table the new nrow becomes the fist column added if (isNull(rows)) { @@ -665,6 +666,12 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values) PROTECT(nullint=allocVector(INTSXP, 0)); protecti++; setAttrib(dt, R_RowNamesSymbol, nullint); // i.e. .set_row_names(0) } + } else if (is_null_dt) { // fix for 4597; null.data.table needs row names + SEXP rn; + PROTECT(rn = allocVector(INTSXP, 2)); protecti++; + INTEGER(rn)[0] = NA_INTEGER; + INTEGER(rn)[1] = -nrow; + setAttrib(dt, R_RowNamesSymbol, rn); } UNPROTECT(protecti); return(dt); // needed for `*tmp*` mechanism (when := isn't used), and to return the new object after a := for compound syntax. From f73477762ca45cc76b21b6dcbcdca3cacc0b663f Mon Sep 17 00:00:00 2001 From: Cole Miller <57992489+ColeMiller1@users.noreply.github.com> Date: Thu, 16 Jul 2020 23:02:21 -0400 Subject: [PATCH 02/14] Update tests.Rraw --- inst/tests/tests.Rraw | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 21caa1fd26..045e53b856 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -17076,3 +17076,10 @@ test(2150.17, fread("a,b,c\n2015-01-01,2015-01-02,2015-01-03 01:02:03", colClass test(2150.18, fread("a,b,c\n2015-01-01,2015-01-02,2015-01-03 01:02:03", colClasses=c("Date",NA,NA)), data.table(a=as.Date("2015-01-01"), b=as.IDate("2015-01-02"), c="2015-01-03 01:02:03"), output=ans_print) options(old) + +## Update row.names for a null.data.table on set #4597 +dt = data.table() +test(2151.1, attr(dt[, x := 1:5], "row.names"), 1:5) +dt = data.table() +set(dt, j = c("v1", "v2"), value = list(1:6, 2:7)) +test(2151.2, attr(dt, "row.names"), 1:6) From 4e0bb3965dffbd9e094dab99129e87f56df00bab Mon Sep 17 00:00:00 2001 From: Cole Miller <57992489+ColeMiller1@users.noreply.github.com> Date: Tue, 21 Jul 2020 19:35:07 -0400 Subject: [PATCH 03/14] Throw warning if !is.null.data.table && nrow(dt) == 0 && nrow(values) > 0 --- src/assign.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/assign.c b/src/assign.c index c9f83ac638..d46f8de4c1 100644 --- a/src/assign.c +++ b/src/assign.c @@ -318,10 +318,12 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values) error(_("data.table is NULL; malformed. A null data.table should be an empty list. typeof() should always return 'list' for data.table.")); // # nocov // Not possible to test because R won't permit attributes be attached to NULL (which is good and we like); warning from R 3.4.0+ tested by 944.5 } - const bool is_null_dt = LENGTH(dt) == 0; // fix for 4597; null.data.table needs row names + const bool is_null_dt = LENGTH(dt) == 0; // fix for #4597; null.data.table needs row names const int nrow = !is_null_dt ? length(VECTOR_ELT(dt,0)) : (isNewList(values) && length(values) ? length(VECTOR_ELT(values,0)) : length(values)); // ^ when null data.table the new nrow becomes the fist column added + if (!is_null_dt && nrow == 0 && RHS_row > 0) // related to #4597; groupingsets relies on this so right now only a warning + warning(_("The data.table has 0 rows but there are %d rows being assigned. This warning will be upgraded to error in next release"), RHS_row); if (isNull(rows)) { numToDo = nrow; targetlen = nrow; @@ -666,7 +668,7 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values) PROTECT(nullint=allocVector(INTSXP, 0)); protecti++; setAttrib(dt, R_RowNamesSymbol, nullint); // i.e. .set_row_names(0) } - } else if (is_null_dt) { // fix for 4597; null.data.table needs row names + } else if (is_null_dt) { // fix for #4597; null.data.table needs row names SEXP rn; PROTECT(rn = allocVector(INTSXP, 2)); protecti++; INTEGER(rn)[0] = NA_INTEGER; From 5b0d7b9224fe2563572b981159cd3f081cd62e4f Mon Sep 17 00:00:00 2001 From: Cole Miller <57992489+ColeMiller1@users.noreply.github.com> Date: Tue, 21 Jul 2020 19:53:35 -0400 Subject: [PATCH 04/14] Update groupingsets.R --- R/groupingsets.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/groupingsets.R b/R/groupingsets.R index 6281615dd5..68b617dcee 100644 --- a/R/groupingsets.R +++ b/R/groupingsets.R @@ -91,7 +91,7 @@ groupingsets.data.table = function(x, j, by, sets, .SDcols, id = FALSE, jj, ...) # aggregate function called for each grouping set aggregate.set = function(by.set) { r = if (length(.SDcols)) x[, eval(jj), by.set, .SDcols=.SDcols] else x[, eval(jj), by.set] - if (id) { + if (id && nrow(r) > 0L) { # #4597 ; warning when using set on nrow(dt) == 0L # integer bit mask of aggregation levels: http://www.postgresql.org/docs/9.5/static/functions-aggregate.html#FUNCTIONS-GROUPING-TABLE # 3267: strtoi("", base = 2L) output apparently unstable across platforms i_str = paste(c("1", "0")[by %chin% by.set + 1L], collapse="") From 2e7b21c465947b7a347441be5b959f8a87fec67f Mon Sep 17 00:00:00 2001 From: Cole Miller <57992489+ColeMiller1@users.noreply.github.com> Date: Tue, 21 Jul 2020 20:09:25 -0400 Subject: [PATCH 05/14] forgot rhs_row declare... --- src/assign.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/assign.c b/src/assign.c index d46f8de4c1..23e7bdef8f 100644 --- a/src/assign.c +++ b/src/assign.c @@ -318,6 +318,7 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values) error(_("data.table is NULL; malformed. A null data.table should be an empty list. typeof() should always return 'list' for data.table.")); // # nocov // Not possible to test because R won't permit attributes be attached to NULL (which is good and we like); warning from R 3.4.0+ tested by 944.5 } + const int RHS_row = isNewList(values) && length(values) ? length(VECTOR_ELT(values, 0)) : length(values); const bool is_null_dt = LENGTH(dt) == 0; // fix for #4597; null.data.table needs row names const int nrow = !is_null_dt ? length(VECTOR_ELT(dt,0)) : (isNewList(values) && length(values) ? length(VECTOR_ELT(values,0)) : length(values)); From cde5c81e111a532face1f770c2a87def9dcb7078 Mon Sep 17 00:00:00 2001 From: Cole Miller <57992489+ColeMiller1@users.noreply.github.com> Date: Tue, 21 Jul 2020 20:22:05 -0400 Subject: [PATCH 06/14] Update tests.Rraw --- inst/tests/tests.Rraw | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index d26834e8fa..80bc285c53 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -6000,20 +6000,20 @@ test(1419.11, key(thisDT), NULL) ## same tests for empty DT ## forderv tests can be skipped for empty DT DT <- DT[0] -thisDT <- copy(DT)[, x3 := 3] +thisDT <- copy(DT)[, x3 := numeric()] test(1419.12, key(thisDT), NULL) setkeyv(DT, c("x1", "x2", "x3")) -thisDT <- copy(DT)[, x1 := 3] +thisDT <- copy(DT)[, x1 := numeric()] test(1419.13, key(thisDT), NULL) -thisDT <- copy(DT)[, x2 := 3] +thisDT <- copy(DT)[, x2 := numeric()] test(1419.14, key(thisDT), "x1") -thisDT <- copy(DT)[, x2 := 3] +thisDT <- copy(DT)[, x2 := numeric()] test(1419.15, key(thisDT), "x1") -thisDT <- copy(DT)[, x3 := 3] +thisDT <- copy(DT)[, x3 := numeric()] test(1419.16, key(thisDT), c("x1", "x2")) -thisDT <- copy(DT)[, c("x1", "x3") := .(3,3)] +thisDT <- copy(DT)[, c("x1", "x3") := .(numeric(), numeric())] test(1419.17, key(thisDT), NULL) -thisDT <- copy(DT)[, c("x2", "x3") := .(3,3)] +thisDT <- copy(DT)[, c("x2", "x3") := .(numeric(),numeric())] test(1419.18, key(thisDT), "x1") ## testing secondary index retainment on assign (#2372) @@ -6094,25 +6094,25 @@ test(1419.46, allIndicesValid(thisDT), TRUE) ## on empty DT DT <- DT[0] setindex(DT, NULL) -test(1419.47, indices(copy(DT)[, a:=1]), NULL) +test(1419.47, indices(copy(DT)[, a:=numeric()]), NULL) setindex(DT, a) setindex(DT, a, aaa) setindex(DT, ab, aaa) setindex(DT) test(1419.48, allIndicesValid(DT), TRUE) -thisDT <- copy(DT)[, a:=1][, aaa := 1][, ab := 1] +thisDT <- copy(DT)[, a:=numeric()][, aaa := numeric()][, ab := numeric()] test(1419.49, indices(thisDT), NULL) test(1419.51, allIndicesValid(thisDT), TRUE) -thisDT <- copy(DT)[, b := 2] +thisDT <- copy(DT)[, b := numeric()] test(1419.52, indices(thisDT), c("a", "a__aaa", "ab__aaa")) test(1419.53, allIndicesValid(thisDT), TRUE) -thisDT <- copy(DT)[, ab := 2] +thisDT <- copy(DT)[, ab := numeric()] test(1419.54, indices(thisDT), c("a", "a__aaa", "a__aaa__b")) test(1419.55, allIndicesValid(thisDT), TRUE) -thisDT <- copy(DT)[, aaa := 2] +thisDT <- copy(DT)[, aaa := numeric()] test(1419.56, indices(thisDT), c("a", "ab")) test(1419.57, allIndicesValid(thisDT), TRUE) -thisDT <- copy(DT)[, c("aaa", "b") := 2] +thisDT <- copy(DT)[, c("aaa", "b") := numeric()] test(1419.58, indices(thisDT), c("a", "ab")) test(1419.59, allIndicesValid(thisDT), TRUE) From ff6482f4ec0a7ac28489f291a2c4987f69c7af8a Mon Sep 17 00:00:00 2001 From: Cole Miller <57992489+ColeMiller1@users.noreply.github.com> Date: Tue, 21 Jul 2020 21:14:28 -0400 Subject: [PATCH 07/14] Coverage --- inst/tests/tests.Rraw | 2 ++ 1 file changed, 2 insertions(+) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 80bc285c53..addb48f09e 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -17120,3 +17120,5 @@ test(2153.1, attr(dt[, x := 1:5], "row.names"), 1:5) dt = data.table() set(dt, j = c("v1", "v2"), value = list(1:6, 2:7)) test(2153.2, attr(dt, "row.names"), 1:6) +dt = data.table(x = integer()) +test(2153.3, dt[, y := 3L], warning = "The data.table has 0 rows but there are 1 row(s) being assigned. The value will be ignored but new column(s) will be added. This warning will be upgraded to error in the next release") From 699eaeff4a1e17f2a2f50bfce51ee444373a0b65 Mon Sep 17 00:00:00 2001 From: Cole Miller <57992489+ColeMiller1@users.noreply.github.com> Date: Tue, 21 Jul 2020 21:15:37 -0400 Subject: [PATCH 08/14] Update warning message --- src/assign.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/assign.c b/src/assign.c index 23e7bdef8f..7013e39bae 100644 --- a/src/assign.c +++ b/src/assign.c @@ -324,7 +324,7 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values) (isNewList(values) && length(values) ? length(VECTOR_ELT(values,0)) : length(values)); // ^ when null data.table the new nrow becomes the fist column added if (!is_null_dt && nrow == 0 && RHS_row > 0) // related to #4597; groupingsets relies on this so right now only a warning - warning(_("The data.table has 0 rows but there are %d rows being assigned. This warning will be upgraded to error in next release"), RHS_row); + warning(_("The data.table has 0 rows but there are %d rows being assigned. The value will be ignored but new column(s) will be added. This warning will be upgraded to error in the next release"), RHS_row); if (isNull(rows)) { numToDo = nrow; targetlen = nrow; From 42c27b11cd136b0a4f2fb1e6a18c281eb4aaa78a Mon Sep 17 00:00:00 2001 From: Cole Miller <57992489+ColeMiller1@users.noreply.github.com> Date: Tue, 21 Jul 2020 21:53:51 -0400 Subject: [PATCH 09/14] Update assign.c --- src/assign.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/assign.c b/src/assign.c index 7013e39bae..9e44d21ba5 100644 --- a/src/assign.c +++ b/src/assign.c @@ -324,7 +324,7 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values) (isNewList(values) && length(values) ? length(VECTOR_ELT(values,0)) : length(values)); // ^ when null data.table the new nrow becomes the fist column added if (!is_null_dt && nrow == 0 && RHS_row > 0) // related to #4597; groupingsets relies on this so right now only a warning - warning(_("The data.table has 0 rows but there are %d rows being assigned. The value will be ignored but new column(s) will be added. This warning will be upgraded to error in the next release"), RHS_row); + warning(_("The data.table has 0 rows but there are %d row(s) being assigned. The value will be ignored but new column(s) will be added. This warning will be upgraded to error in the next release"), RHS_row); if (isNull(rows)) { numToDo = nrow; targetlen = nrow; From 85b85815701d4d5cde89377caf137e1ef087e6de Mon Sep 17 00:00:00 2001 From: Cole Miller <57992489+ColeMiller1@users.noreply.github.com> Date: Thu, 23 Jul 2020 21:28:25 -0400 Subject: [PATCH 10/14] Update tests.Rraw --- inst/tests/tests.Rraw | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index addb48f09e..eace6e6b2c 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -17121,4 +17121,4 @@ dt = data.table() set(dt, j = c("v1", "v2"), value = list(1:6, 2:7)) test(2153.2, attr(dt, "row.names"), 1:6) dt = data.table(x = integer()) -test(2153.3, dt[, y := 3L], warning = "The data.table has 0 rows but there are 1 row(s) being assigned. The value will be ignored but new column(s) will be added. This warning will be upgraded to error in the next release") +test(2153.3, dt[, y := 3L], data.table(x = integer(), y = integer()), warning = "The data.table has 0 rows but there are 1 row(s) being assigned. The value will be ignored but new column(s) will be added. This warning will be upgraded to error in the next release") From 58d1a4cdf93b2429a030b33e39a9add27435c059 Mon Sep 17 00:00:00 2001 From: Cole Miller <57992489+ColeMiller1@users.noreply.github.com> Date: Tue, 2 Feb 2021 22:38:41 -0500 Subject: [PATCH 11/14] Fix typo --- inst/tests/tests.Rraw | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 9e714112e3..117123cd14 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -17267,6 +17267,6 @@ dt = data.table() test(2164.1, attr(dt[, x := 1:5], "row.names"), 1:5) dt = data.table() set(dt, j = c("v1", "v2"), value = list(1:6, 2:7)) -test(21564.2, attr(dt, "row.names"), 1:6) +test(2164.2, attr(dt, "row.names"), 1:6) dt = data.table(x = integer()) test(2164.3, dt[, y := 3L], data.table(x = integer(), y = integer()), warning = "The data.table has 0 rows but there are 1 row(s) being assigned. The value will be ignored but new column(s) will be added. This warning will be upgraded to error in the next release") From 3508d1fb0333f0eec0e0f8c2e9442e6c2daea461 Mon Sep 17 00:00:00 2001 From: Matt Dowle Date: Thu, 29 Apr 2021 14:31:08 -0600 Subject: [PATCH 12/14] removed new warning, moved fix, added news item --- NEWS.md | 2 ++ R/groupingsets.R | 2 +- inst/tests/tests.Rraw | 29 +++++++++++++++-------------- src/assign.c | 20 +++++++++----------- 4 files changed, 27 insertions(+), 26 deletions(-) diff --git a/NEWS.md b/NEWS.md index 9b179489ce..762f18e948 100644 --- a/NEWS.md +++ b/NEWS.md @@ -63,6 +63,8 @@ 8. `rbind()` and `rbindlist()` of length-0 ordered factors failed with `Internal error: savetl_init checks failed`, [#4795](https://github.com/Rdatatable/data.table/issues/4795) [#4823](https://github.com/Rdatatable/data.table/issues/4823). Thanks to @shrektan and @dbart79 for reporting, and @shrektan for fixing. +9. `data.table(NULL)[, firstCol:=1L]` created `data.table(firstCol=1L)` ok but did not update the internal `row.names` attribute, causing `Error in `$<-.data.frame`(x, name, value) : replacement has 1 row, data has 0` when passed to packages like `ggplot` which use `DT` as if it is a `data.frame`, [#4597](https://github.com/Rdatatable/data.table/issues/4597). Thanks to Matthew Son for reporting, and Cole Miller 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/R/groupingsets.R b/R/groupingsets.R index 81337635b1..5c3ad02d4b 100644 --- a/R/groupingsets.R +++ b/R/groupingsets.R @@ -93,7 +93,7 @@ groupingsets.data.table = function(x, j, by, sets, .SDcols, id = FALSE, jj, ...) # aggregate function called for each grouping set aggregate.set = function(by.set) { r = if (length(.SDcols)) x[, eval(jj), by.set, .SDcols=.SDcols] else x[, eval(jj), by.set] - if (id && nrow(r) > 0L) { # #4597 ; warning when using set on nrow(dt) == 0L + if (id) { # integer bit mask of aggregation levels: http://www.postgresql.org/docs/9.5/static/functions-aggregate.html#FUNCTIONS-GROUPING-TABLE # 3267: strtoi("", base = 2L) output apparently unstable across platforms i_str = paste(c("1", "0")[by %chin% by.set + 1L], collapse="") diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 2b58847482..bcdd90f6af 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -6019,20 +6019,20 @@ test(1419.11, key(thisDT), NULL) ## same tests for empty DT ## forderv tests can be skipped for empty DT DT <- DT[0] -thisDT <- copy(DT)[, x3 := numeric()] +thisDT <- copy(DT)[, x3 := 3] test(1419.12, key(thisDT), NULL) setkeyv(DT, c("x1", "x2", "x3")) -thisDT <- copy(DT)[, x1 := numeric()] +thisDT <- copy(DT)[, x1 := 3] test(1419.13, key(thisDT), NULL) -thisDT <- copy(DT)[, x2 := numeric()] +thisDT <- copy(DT)[, x2 := 3] test(1419.14, key(thisDT), "x1") -thisDT <- copy(DT)[, x2 := numeric()] +thisDT <- copy(DT)[, x2 := 3] test(1419.15, key(thisDT), "x1") -thisDT <- copy(DT)[, x3 := numeric()] +thisDT <- copy(DT)[, x3 := 3] test(1419.16, key(thisDT), c("x1", "x2")) -thisDT <- copy(DT)[, c("x1", "x3") := .(numeric(), numeric())] +thisDT <- copy(DT)[, c("x1", "x3") := .(3,3)] test(1419.17, key(thisDT), NULL) -thisDT <- copy(DT)[, c("x2", "x3") := .(numeric(),numeric())] +thisDT <- copy(DT)[, c("x2", "x3") := .(3,3)] test(1419.18, key(thisDT), "x1") ## testing secondary index retainment on assign (#2372) @@ -6113,25 +6113,25 @@ test(1419.46, allIndicesValid(thisDT), TRUE) ## on empty DT DT <- DT[0] setindex(DT, NULL) -test(1419.47, indices(copy(DT)[, a:=numeric()]), NULL) +test(1419.47, indices(copy(DT)[, a:=1]), NULL) setindex(DT, a) setindex(DT, a, aaa) setindex(DT, ab, aaa) setindex(DT) test(1419.48, allIndicesValid(DT), TRUE) -thisDT <- copy(DT)[, a:=numeric()][, aaa := numeric()][, ab := numeric()] +thisDT <- copy(DT)[, a:=1][, aaa := 1][, ab := 1] test(1419.49, indices(thisDT), NULL) test(1419.51, allIndicesValid(thisDT), TRUE) -thisDT <- copy(DT)[, b := numeric()] +thisDT <- copy(DT)[, b := 2] test(1419.52, indices(thisDT), c("a", "a__aaa", "ab__aaa")) test(1419.53, allIndicesValid(thisDT), TRUE) -thisDT <- copy(DT)[, ab := numeric()] +thisDT <- copy(DT)[, ab := 2] test(1419.54, indices(thisDT), c("a", "a__aaa", "a__aaa__b")) test(1419.55, allIndicesValid(thisDT), TRUE) -thisDT <- copy(DT)[, aaa := numeric()] +thisDT <- copy(DT)[, aaa := 2] test(1419.56, indices(thisDT), c("a", "ab")) test(1419.57, allIndicesValid(thisDT), TRUE) -thisDT <- copy(DT)[, c("aaa", "b") := numeric()] +thisDT <- copy(DT)[, c("aaa", "b") := 2] test(1419.58, indices(thisDT), c("a", "ab")) test(1419.59, allIndicesValid(thisDT), TRUE) @@ -17389,4 +17389,5 @@ DT = data.table() set(DT, j=c("v1","v2"), value=list(1:6, 2:7)) test(2175.2, attr(DT, "row.names"), 1:6) DT = data.table(x=integer()) -test(2175.3, DT[, y:=3L], data.table(x=integer(), y=integer()), warning="has 0 rows but.*1.*being assigned") +test(2175.3, DT[, y:=3L], data.table(x=integer(), y=integer())) # in keeping with recent #4262, view as recycling the length-1 3L to match the length-0 data + diff --git a/src/assign.c b/src/assign.c index e29e431f48..e811276610 100644 --- a/src/assign.c +++ b/src/assign.c @@ -318,13 +318,9 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values) error(_("data.table is NULL; malformed. A null data.table should be an empty list. typeof() should always return 'list' for data.table.")); // # nocov // Not possible to test because R won't permit attributes be attached to NULL (which is good and we like); warning from R 3.4.0+ tested by 944.5 } - const int RHS_row = isNewList(values) && length(values) ? length(VECTOR_ELT(values, 0)) : length(values); - const bool is_null_dt = LENGTH(dt) == 0; // fix for #4597; null.data.table needs row names - const int nrow = !is_null_dt ? length(VECTOR_ELT(dt,0)) : + const int nrow = LENGTH(dt) ? length(VECTOR_ELT(dt,0)) : (isNewList(values) && length(values) ? length(VECTOR_ELT(values,0)) : length(values)); // ^ when null data.table the new nrow becomes the fist column added - if (!is_null_dt && nrow == 0 && RHS_row > 0) // related to #4597; groupingsets relies on this so right now only a warning - warning(_("The data.table has 0 rows but there are %d row(s) being assigned. The value will be ignored but new column(s) will be added. This warning will be upgraded to error in the next release"), RHS_row); if (isNull(rows)) { numToDo = nrow; targetlen = nrow; @@ -477,6 +473,14 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values) for (i=0; i Date: Thu, 29 Apr 2021 14:32:59 -0600 Subject: [PATCH 13/14] backquotes in news item --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 762f18e948..45fbd888bb 100644 --- a/NEWS.md +++ b/NEWS.md @@ -63,7 +63,7 @@ 8. `rbind()` and `rbindlist()` of length-0 ordered factors failed with `Internal error: savetl_init checks failed`, [#4795](https://github.com/Rdatatable/data.table/issues/4795) [#4823](https://github.com/Rdatatable/data.table/issues/4823). Thanks to @shrektan and @dbart79 for reporting, and @shrektan for fixing. -9. `data.table(NULL)[, firstCol:=1L]` created `data.table(firstCol=1L)` ok but did not update the internal `row.names` attribute, causing `Error in `$<-.data.frame`(x, name, value) : replacement has 1 row, data has 0` when passed to packages like `ggplot` which use `DT` as if it is a `data.frame`, [#4597](https://github.com/Rdatatable/data.table/issues/4597). Thanks to Matthew Son for reporting, and Cole Miller for the PR. +9. `data.table(NULL)[, firstCol:=1L]` created `data.table(firstCol=1L)` ok but did not update the internal `row.names` attribute, causing 'Error in $<-.data.frame'(x, name, value) : replacement has 1 row, data has 0` when passed to packages like `ggplot` which use `DT` as if it is a `data.frame`, [#4597](https://github.com/Rdatatable/data.table/issues/4597). Thanks to Matthew Son for reporting, and Cole Miller for the PR. ## NOTES From 7a5263b73e83685226b9d08d7c1c8020ac5ed32f Mon Sep 17 00:00:00 2001 From: Matt Dowle Date: Thu, 29 Apr 2021 14:33:44 -0600 Subject: [PATCH 14/14] backquotes in news item again --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 45fbd888bb..8cbe8b2f7b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -63,7 +63,7 @@ 8. `rbind()` and `rbindlist()` of length-0 ordered factors failed with `Internal error: savetl_init checks failed`, [#4795](https://github.com/Rdatatable/data.table/issues/4795) [#4823](https://github.com/Rdatatable/data.table/issues/4823). Thanks to @shrektan and @dbart79 for reporting, and @shrektan for fixing. -9. `data.table(NULL)[, firstCol:=1L]` created `data.table(firstCol=1L)` ok but did not update the internal `row.names` attribute, causing 'Error in $<-.data.frame'(x, name, value) : replacement has 1 row, data has 0` when passed to packages like `ggplot` which use `DT` as if it is a `data.frame`, [#4597](https://github.com/Rdatatable/data.table/issues/4597). Thanks to Matthew Son for reporting, and Cole Miller for the PR. +9. `data.table(NULL)[, firstCol:=1L]` created `data.table(firstCol=1L)` ok but did not update the internal `row.names` attribute, causing `Error in '$<-.data.frame'(x, name, value) : replacement has 1 row, data has 0` when passed to packages like `ggplot` which use `DT` as if it is a `data.frame`, [#4597](https://github.com/Rdatatable/data.table/issues/4597). Thanks to Matthew Son for reporting, and Cole Miller for the PR. ## NOTES