Skip to content

Commit

Permalink
Fix for #481 (and #734, duplicate). Fix for ":=" that was missed earl…
Browse files Browse the repository at this point in the history
…ier.
  • Loading branch information
arunsrinivasan committed Jul 17, 2014
1 parent 090a2b2 commit 5e2ced0
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 2 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ We moved from R-Forge to GitHub on 9 June 2014, including history.
32. Fixed an edge case in `DT[order(.)]` internal optimisation to be consistent with base. Closes [#696](https://github.com/Rdatatable/data.table/issues/696). Thanks to Michael Smith and Garrett See for reporting.
33. `DT[, list(list(.)), by=.]` returns correct results in R >=3.1.0 as well. The bug was due to recent (welcoming) changes in R v3.1.0 where `list(.)` does not result in a *copy*. Closes [#481](https://github.com/Rdatatable/data.table/issues/481).
33. `DT[, list(list(.)), by=.]` and `DT[, col := list(list(.)), by=.]` returns correct results in R >=3.1.0 as well. The bug was due to recent (welcoming) changes in R v3.1.0 where `list(.)` does not result in a *copy*. Closes [#481](https://github.com/Rdatatable/data.table/issues/481). Also thanks to KrishnaPG for filing [#728](https://github.com/Rdatatable/data.table/issues/728).
34. `dcast.data.table` handles `fun.aggregate` argument properly when called from within a function that accepts `fun.aggregate` argument and passes to `dcast.data.table()`. Closes [#713](https://github.com/Rdatatable/data.table/issues/713). Thanks to mathematicalcoffee for reporting [here](http://stackoverflow.com/q/24542976/559784) on SO.
Expand Down
6 changes: 5 additions & 1 deletion inst/tests/tests.Rraw
Original file line number Diff line number Diff line change
Expand Up @@ -4838,11 +4838,15 @@ test(1340.18, setorderv(copy(DT), "x", na.last=NA), error="na.last must be logic

# bug #481 - DT[, list(list(.)), by=.] on R v3.1.0
set.seed(1L)
f <- function(x) list(x)
DT <- data.table(x=sample(3,10,TRUE), y=as.numeric(sample(10)))
test(1341.1, DT[, list(list(y)), by=x], data.table(x=unique(DT$x), V1=list(c(3,5,9), c(2,6,4,1), c(10,7,8))))
test(1341.2, DT[, list(list(.I)), by=x], data.table(x=unique(DT$x), V1=list(c(1,5,10), c(2,3,8,9), c(4,6,7))))
f <- function(x) list(x)
test(1341.3, DT[, list(f(y)), by=x], data.table(x=unique(DT$x), V1=list(c(3,5,9), c(2,6,4,1), c(10,7,8))))
# test for list(list(.)) with :=
test(1341.4, copy(DT)[, z := list(list(y)), by=x], copy(DT)[, z := list(list(copy(y))), by=x])
test(1341.5, copy(DT)[, z := list(list(.I)), by=x], copy(DT)[, z := list(list(copy(.I))), by=x])
test(1341.6, copy(DT)[, z := list(f(y)), by=x], copy(DT)[, z := list(f(copy(y))), by=x])

# test regression on over-allocation (selfref) on unique() which uses new subsetDT()
bla <- data.table(x=c(1,1,2,2), y=c(1,1,1,1))
Expand Down
22 changes: 22 additions & 0 deletions src/dogroups.c
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,29 @@ SEXP dogroups(SEXP dt, SEXP dtcols, SEXP groups, SEXP grpcols, SEXP jiscols, SEX
// fix for #4990 - `:=` did not issue recycling warning during "by" operation.
if (vlen<grpn && vlen>0 && grpn%vlen != 0)
warning("Supplied %d items to be assigned to group %d of size %d in column '%s' (recycled leaving remainder of %d items).",vlen,i+1,grpn,CHAR(STRING_ELT(dtnames,INTEGER(lhs)[j]-1)),grpn%vlen);
// fix for issues/481 for := case
// missed it in commit: https://github.com/Rdatatable/data.table/commit/86276f48798491d328caa72f6ebcce4d51649440
// see that link (or scroll down for the non := version) for comments
#if defined(R_VERSION) && R_VERSION >= R_Version(3, 1, 0)
named=0;
if (isNewList(RHS) && NAMED(RHS) != 2) {
dupcol = VECTOR_ELT(RHS, 0);
named = NAMED(dupcol);
while(isNewList(dupcol)) {
if (named == 2) break;
else {
dupcol = VECTOR_ELT(dupcol, 0);
named = NAMED(dupcol);
}
}
if (named == 2) RHS = PROTECT(duplicate(RHS));
}
memrecycle(target, order, INTEGER(starts)[i]-1, grpn, RHS);
if (named == 2) UNPROTECT(1);
#else
memrecycle(target, order, INTEGER(starts)[i]-1, grpn, RHS);
#endif

// fixes bug #2531. Got to set the class back. See comment below for explanation. This is the new fix. Works great!
// Also fix for #5437 (bug due to regression in 1.9.2+)
copyMostAttrib(RHS, target); // not names, otherwise test 778 would fail
Expand Down

0 comments on commit 5e2ced0

Please sign in to comment.