diff --git a/NEWS.md b/NEWS.md index bf4250b16..1cfd582f7 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,10 @@ # data.table [v1.14.99](https://github.com/Rdatatable/data.table/milestone/20) (in development) +## BREAKING CHANGE + +1. `shift` and `nafill` will now raise error `input must not be matrix or array` when `matrix` or `array` is provided on input, rather than giving useless result, [#5287](https://github.com/Rdatatable/data.table/issues/5287). Thanks to @ethanbsmith for reporting. + ## NEW FEATURES 1. `nafill()` now applies `fill=` to the front/back of the vector when `type="locf|nocb"`, [#3594](https://github.com/Rdatatable/data.table/issues/3594). Thanks to @ben519 for the feature request. It also now returns a named object based on the input names. Note that if you are considering joining and then using `nafill(...,type='locf|nocb')` afterwards, please review `roll=`/`rollends=` which should achieve the same result in one step more efficiently. `nafill()` is for when filling-while-joining (i.e. `roll=`/`rollends=`/`nomatch=`) cannot be applied. diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 98d81fe2b..8eeb8f7ee 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -18108,3 +18108,6 @@ test(2238.6, "a" %notin% integer(), TRUE) test(2238.7, "a" %notin% NULL, TRUE) test(2238.8, NA %notin% 1:5, TRUE) test(2238.9, NA %notin% c(1:5, NA), FALSE) + +# shift actionable error on matrix input #5287 +test(2239.1, shift(matrix(1:10, ncol = 1)), error="consider wrapping") diff --git a/src/shift.c b/src/shift.c index dba598fe5..30c13a547 100644 --- a/src/shift.c +++ b/src/shift.c @@ -8,6 +8,8 @@ SEXP shift(SEXP obj, SEXP k, SEXP fill, SEXP type) if (!xlength(obj)) return(obj); // NULL, list() SEXP x; if (isVectorAtomic(obj)) { + if (!isNull(getAttrib(obj, R_DimSymbol))) + error(_("shift input must not be matrix or array, consider wrapping it into data.table() or c()")); x = PROTECT(allocVector(VECSXP, 1)); nprotect++; SET_VECTOR_ELT(x, 0, obj); } else { diff --git a/src/utils.c b/src/utils.c index 3dfd8bcc6..e5e343ac9 100644 --- a/src/utils.c +++ b/src/utils.c @@ -348,7 +348,7 @@ SEXP coerceAs(SEXP x, SEXP as, SEXP copyArg) { if (!isNull(getAttrib(x, R_DimSymbol))) error(_("'x' must not be matrix or array")); if (!isNull(getAttrib(as, R_DimSymbol))) - error(_("'as' must not be matrix or array")); + error(_("input must not be matrix or array")); bool verbose = GetVerbose()>=2; // verbose level 2 required if (!LOGICAL(copyArg)[0] && TYPEOF(x)==TYPEOF(as) && class1(x)==class1(as)) { if (verbose)