Apply Ben's suggested changes

Co-authored-by: Benjamin Schwendinger <[email protected]>

Author: MichaelChirico

R/mergelist.R

@@ -2,7 +2,7 @@ cbindlist = function(l, copy=TRUE) {
   ans = .Call(Ccbindlist, l, copy)
   if (anyDuplicated(names(ans))) { ## invalidate key and index
     setattr(ans, "sorted", NULL)
-    setattr(ans, "index", integer())
+    setattr(ans, "index", NULL)
   }
   setDT(ans)
   ans

inst/tests/mergelist.Rraw

@@ -70,3 +70,6 @@ ans = cbindlist(l)
 test(13.04, key(ans), "id1")
 test(13.05, indices(ans), c("id1","id2","id3","id1__id2__id3","id6","id7","id9"))
 test(13.06, ii, lapply(l, indices)) ## this tests that original indices have not been touched, shallow_duplicate in mergeIndexAttrib
+ans = cbindlist(list(data.table(a=1:2), data.table(b=3:4, key="b")))
+test(13.07, ans, data.table(a=1:2, b=3:4, key="b"))
+test(13.08, key(ans), "b")

man/cbindlist.Rd

@@ -26,11 +26,13 @@
   \code{\link{data.table}}, \code{\link{rbindlist}}
 }
 \examples{
-l = list(
-  d1 = data.table(x=1:3, v1=1L),
-  d2 = data.table(y=3:1, v2=2L),
-  d3 = data.table(z=2:4, v3=3L)
-)
-cbindlist(l)
+d1 = data.table(x=1:3, v1=1L, key="x")
+d2 = data.table(y=3:1, v2=2L, key="y")
+d3 = data.table(z=2:4, v3=3L)
+cbindlist(list(d1, d2, d3))
+cbindlist(list(d1, d1))
+d4 = cbindlist(list(d1), copy=FALSE)
+d4[, v1:=2L]
+identical(d4, d1)
 }
 \keyword{ data }

src/mergelist.c

@@ -6,15 +6,14 @@ void mergeIndexAttrib(SEXP to, SEXP from) {
   if (isNull(from))
     return;
   SEXP t = ATTRIB(to), f = ATTRIB(from);
-  if (isNull(f))
+  if (isNull(f))  // nothing to merge
     return;
-  if (isNull(t))
+  if (isNull(t)) // target has no attributes -> overwrite
     SET_ATTRIB(to, shallow_duplicate(f));
   else {
-    for (t = ATTRIB(to); CDR(t) != R_NilValue; t = CDR(t));
+    for (t = ATTRIB(to); CDR(t) != R_NilValue; t = CDR(t)); // traverse to end of attributes list of to
     SETCDR(t, shallow_duplicate(f));
   }
-  return;
 }
 
 SEXP cbindlist(SEXP x, SEXP copyArg) {
@@ -72,6 +71,8 @@ SEXP cbindlist(SEXP x, SEXP copyArg) {
       key = getAttrib(thisx, sym_sorted);
     UNPROTECT(protecti);
   }
+  if (isNull(ATTRIB(index)))
+    setAttrib(ans, sym_index, R_NilValue);
   setAttrib(ans, R_NamesSymbol, names);
   setAttrib(ans, sym_sorted, key);
   if (verbose)