From 376e15a6a7a8ec3438ab23dafa505e833a043c0f Mon Sep 17 00:00:00 2001 From: mattdowle Date: Wed, 22 May 2019 15:50:15 -0700 Subject: [PATCH] recyle error to existing list column --- NEWS.md | 2 ++ inst/tests/tests.Rraw | 17 ++++++++++++----- src/assign.c | 2 +- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/NEWS.md b/NEWS.md index 8dc12a15f4..342ee0fbf7 100644 --- a/NEWS.md +++ b/NEWS.md @@ -110,6 +110,8 @@ 12. Joining a double column in `i` containing say 1.3, with an integer column in `x` containing say 1, would result in the 1.3 matching to 1, [#2592](https://github.com/Rdatatable/data.table/issues/2592), and joining a factor column to an integer column would match the factor's integers rather than error. The type coercion logic has been revised and strengthened. Many thanks to @MarkusBonsch for reporting and fixing. Joining a character column in `i` to a factor column in `x` is now faster and retains the character column in the result rather than coercing it to factor. Joining an integer column in `i` to a double column in `x` now retains the integer type in the result rather than coercing the integers into the double type. Logical columns may now only be joined to logical columns, other than all-NA columns which are coerced to the matching column's type. All coercions are reported in verbose mode: `options(datatable.verbose=TRUE)`. +14. Attempting to recycle 2 or more items into an existing `list` column now gives the intended helpful error rather than `Internal error: recycle length error not caught earlier.`, [#3543](https://github.com/Rdatatable/data.table/issues/3543). Thanks to @MichaelChirico for finding and reporting. + #### NOTES 1. `rbindlist`'s `use.names="check"` now emits its message for automatic column names (`"V[0-9]+"`) too, [#3484](https://github.com/Rdatatable/data.table/pull/3484). See news item 5 of v1.12.2 below. diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index d56ee7f1ba..6b2519b416 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -6514,13 +6514,13 @@ if (test_xts) { setcolorder(dt, c(2, 3, 1)) dt[ , char_col := 'a'] test(1465.17, as.xts(dt), xt, warning = 'columns are not numeric') - + # 890 -- key argument for as.data.table.xts x = xts(1:10, as.Date(1:10, origin = "1970-01-01")) test(1465.18, capture.output(as.data.table(x, key="index")), - c(" index V1", " 1: 1970-01-02 1", " 2: 1970-01-03 2", - " 3: 1970-01-04 3", " 4: 1970-01-05 4", " 5: 1970-01-06 5", - " 6: 1970-01-07 6", " 7: 1970-01-08 7", " 8: 1970-01-09 8", + c(" index V1", " 1: 1970-01-02 1", " 2: 1970-01-03 2", + " 3: 1970-01-04 3", " 4: 1970-01-05 4", " 5: 1970-01-06 5", + " 6: 1970-01-07 6", " 7: 1970-01-08 7", " 8: 1970-01-09 8", " 9: 1970-01-10 9", "10: 1970-01-11 10")) Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = TRUE) @@ -14782,7 +14782,7 @@ test(2045.15, d1[d2, verbose = TRUE], cbind(d1, X1 = d2$X1), output="natural joi options(datatable.naturaljoin=FALSE) #tests for adding key to as.data.table, #890 -## as.data.table.numeric (should cover as.data.table.factor, +## as.data.table.numeric (should cover as.data.table.factor, ## *.ordered, *.integer, *.logical, *.character, and *.Date since ## all are the same function in as.data.table.R) nn = c(a=0.1, c=0.2, b=0.3, d=0.4) @@ -14829,6 +14829,13 @@ test(2047.1, as.data.table(list(character(0L))), data.table(V1 = character(0L))) test(2047.2, as.data.table(list()), data.table(NULL)) test(2047.3, as.data.table(rbind(1L)), data.table(V1 = 1L)) +# recyle internal error; #3543 +DT = data.table(A=1:3, existingCol=list(0,1,2)) +test(2048.1, DT[, newCol:=.(rep(0, .N), rep(1, .N))], # was ok before when assigning to new column + error="Supplied 2 items to be assigned to 3 items of column 'newCol'.*please use rep") +test(2048.2, DT[, existingCol:=.(rep(0, .N), rep(1, .N))], # was internal error (rather than helpful error) when assigning to existing column + error="Supplied 2 items to be assigned to 3 items of column 'existingCol'.*please use rep") + ################################### # Add new tests above this line # diff --git a/src/assign.c b/src/assign.c index 75224bf370..7b4973eb2a 100644 --- a/src/assign.c +++ b/src/assign.c @@ -436,7 +436,7 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values, SEXP v error("Can't assign to column '%s' (type 'factor') a value of type '%s' (not character, factor, integer or numeric)", CHAR(STRING_ELT(names,coln)),type2char(TYPEOF(thisvalue))); } - if (nrow>0 && targetlen>0 && vlen>1 && vlen!=targetlen && TYPEOF(existing)!=VECSXP) { + if (nrow>0 && targetlen>0 && vlen>1 && vlen!=targetlen && (TYPEOF(existing)!=VECSXP || TYPEOF(thisvalue)==VECSXP)) { // note that isNewList(R_NilValue) is true so it needs to be TYPEOF(existing)!=VECSXP above error("Supplied %d items to be assigned to %d items of column '%s'. If you wish to 'recycle' the RHS please " "use rep() to make this intent clear to readers of your code.", vlen, targetlen, CHAR(colnam));