diff --git a/NEWS.md b/NEWS.md index e69e99763b..35c6e58acc 100644 --- a/NEWS.md +++ b/NEWS.md @@ -84,6 +84,8 @@ 13. Reference to `.SD` in `...` arguments to `lapply()`, e.g. ``lapply(list_of_tables, `[`, j=.SD[1L])`` is evaluated correctly, [#2982](https://github.com/Rdatatable/data.table/issues/2982). Thanks @franknarf1 for the report and @MichaelChirico for the fix. +14. Filling columns of class Date with POSIXct (and vice versa) using `shift()` now yields a clear, informative error message specifying the class mismatch, [#5218](https://github.com/Rdatatable/data.table/issues/5218). Thanks @ashbaldry for the report and @ben-schwen for the fix. + ### NOTES 1. The following in-progress deprecations have proceeded: diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index bb9416a5a4..ae96bef649 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -6851,6 +6851,13 @@ test(1463.78, shift(x,1:2, type="cyclic"), list(as.raw(c(5, 1:4)), as.raw(c(4: test(1463.79, shift(x,-1L, type="cyclic"), as.raw(c(2:5, 1))) test(1463.80, shift(x,-(1:2),type="cyclic"), list(as.raw(c(2:5, 1)), as.raw(c(3:5,1:2)))) +# shift incompatible types (e.g. Date and POSIXct) +d = .Date(0:4) +p = .POSIXct(1:5) +test(1463.81, shift(d, fill=p[1L]), error="Filling Date with POSIXct .* unsupported.*") +test(1463.82, shift(p, fill=d[1L]), error="Filling POSIXct with Date .* unsupported.*") +test(1463.83, shift(d, fill=as.IDate(2000L)), .Date(c(2000L, 0:3))) + # FR #686 DT = data.table(a=rep(c("A", "B", "C", "A", "B"), c(2,2,3,1,2)), foo=1:10) # Seemingly superfluous 'foo' is needed to test fix for #1942 diff --git a/src/shift.c b/src/shift.c index baefd8b496..33e5756449 100644 --- a/src/shift.c +++ b/src/shift.c @@ -40,6 +40,12 @@ SEXP shift(SEXP obj, SEXP k, SEXP fill, SEXP type) SEXP elem = VECTOR_ELT(x, i); size_t size = RTYPE_SIZEOF(elem); R_xlen_t xrows = xlength(elem); + if (INHERITS(elem, char_Date) && INHERITS(fill, char_POSIXct) || INHERITS(elem, char_POSIXct) && INHERITS(fill, char_Date)) { + const char* elem_type = INHERITS(elem, char_Date) ? "Date" : "POSIXct"; + const char* fill_type = INHERITS(fill, char_Date) ? "Date" : "POSIXct"; + error(_("Filling %s with %s using shift() is unsupported. Please convert fill to %s first."), + elem_type, fill_type, elem_type); + } SEXP thisfill = PROTECT(coerceAs(fill, elem, ScalarLogical(0))); // #4865 use coerceAs for type coercion switch (TYPEOF(elem)) { case INTSXP: case LGLSXP: {