Skip to content

Commit

Permalink
Add list column to null data.table (#5739)
Browse files Browse the repository at this point in the history
* add fix

* update tests

* add more tests

* switch back to cols

* Update NEWS.md

Co-authored-by: Michael Chirico <[email protected]>

* Update src/assign.c

Co-authored-by: Michael Chirico <[email protected]>

* standardize tests

---------

Co-authored-by: Michael Chirico <[email protected]>
  • Loading branch information
ben-schwen and MichaelChirico authored May 19, 2024
1 parent f8b24f8 commit a33b334
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 1 deletion.
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@
7. `melt` returns an integer column for `variable` when `measure.vars` is a list of length=1, consistent with the documented behavior, [#5209](https://github.com/Rdatatable/data.table/issues/5209). Thanks to @tdhock for reporting and fixing. Any users who were relying on this behavior can change `measure.vars=list("col_name")` (output `variable` was column name, now is column index/integer) to `measure.vars="col_name"` (`variable` still is column name).
8. Adding a list column to an empty `data.table` works consistently with other column types, [#5738](https://github.com/Rdatatable/data.table/issues/5738). Thanks to Benjamin Schwendinger for the report and the fix.
## NOTES
1. `transform` method for data.table sped up substantially when creating new columns on large tables. Thanks to @OfekShilon for the report and PR. The implemented solution was proposed by @ColeMiller1.
Expand Down
14 changes: 14 additions & 0 deletions inst/tests/tests.Rraw
Original file line number Diff line number Diff line change
Expand Up @@ -18569,3 +18569,17 @@ test(2261.04, setNumericRounding(2L), 1L)
# or not an object is an invisible copy or not, and prints it anyways.
test(2261.05, capture.output(setNumericRounding(2L)), character(0))
setNumericRounding(old)

# Add list column to null data.table #5738
x = list("a", 1)
dt1 = data.table(a = x)
dt2 = data.table(a = "a", b=1)
dt3 = data.table(a=1:2, b=3:4)
test(2262.1, null.data.table()[, a := x], dt1)
test(2262.2, set(null.data.table(), j="a", value=x), dt1)
test(2262.3, null.data.table()[, c("a","b") := x], dt2)
test(2262.4, set(null.data.table(), j=c("a","b"), value=x), dt2)
test(2262.5, null.data.table()[, c("a","b") := list(1:2, 3:4)], dt3)
test(2262.6, set(null.data.table(), j=c("a","b"), value=list(1:2, 3:4)), dt3)
test(2262.7, data.table(a=1, b=2)[, c("a", "b") := list(NULL, NULL)], null.data.table())
test(2262.8, data.table(a=1, b=2)[, c("a", "b") := list(NULL)], null.data.table())
3 changes: 2 additions & 1 deletion src/assign.c
Original file line number Diff line number Diff line change
Expand Up @@ -360,8 +360,9 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values)
// 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)) :
(isNewList(values) && length(values) ? length(VECTOR_ELT(values,0)) : length(values));
(isNewList(values) && length(values) && (length(values)==length(cols)) ? length(VECTOR_ELT(values,0)) : length(values));
// ^ when null data.table the new nrow becomes the fist column added
// ^ distinguish between adding list column or multiple columns at once, #5738
if (isNull(rows)) {
numToDo = nrow;
targetlen = nrow;
Expand Down

0 comments on commit a33b334

Please sign in to comment.