automatically convert posixlt to posixct in set/:=/<-

NEWS.md

@@ -2,6 +2,10 @@
 
 # data.table [v1.15.99](https://github.com/Rdatatable/data.table/milestone/30)  (in development)
 
+## POTENTIALLY BREAKING CHANGES
+
+1. In `DT[, j := value]`, when value is class `POSIXlt`, we automatically coerce it to class `POSIXct` instead, [#1724](https://github.com/Rdatatable/data.table/issues/1724). Thanks to @linzhp for the report, and Benjamin Schwendinger for the fix.
+
 ## NEW FEATURES
 
 1. `print.data.table()` shows empty (`NULL`) list column entries as `[NULL]` for emphasis. Previously they would just print nothing (same as for empty string). Part of [#4198](https://github.com/Rdatatable/data.table/issues/4198). Thanks @sritchie73 for the proposal and fix.

R/data.table.R

@@ -1268,11 +1268,6 @@ replace_dot_alias = function(e) {
   }  # end of  if !missing(j)
 
   SDenv = new.env(parent=parent.frame())
-  # taking care of warnings for posixlt type, #646
-  SDenv$strptime = function(x, ...) {
-    warningf("strptime() usage detected and wrapped with as.POSIXct(). This is to minimize the chance of assigning POSIXlt columns, which use 40+ bytes to store one date (versus 8 for POSIXct). Use as.POSIXct() (which will call strptime() as needed internally) to avoid this warning.")
-    as.POSIXct(base::strptime(x, ...))
-  }
 
   syms = all.vars(jsub)
   syms = syms[ startsWith(syms, "..") ]

inst/tests/tests.Rraw

@@ -8614,7 +8614,7 @@ ll = list(a=as.POSIXlt("2015-01-01", tz='UTC'), b=1:5)
 test(1612.1, as.data.table(ll), data.table(a=as.POSIXct("2015-01-01", tz='UTC'), b=1:5), warning="POSIXlt column type detected")
 dt = data.table(d1="1984-03-17")
 ans = data.table(d1="1984-03-17", d2=as.POSIXct("1984-03-17", tz='UTC'))
-test(1612.2, dt[, d2 := strptime(d1, "%Y-%m-%d", tz='UTC')], ans, warning="strptime() usage detected and wrapped with as.POSIXct()")
+test(1612.2, dt[, d2 := strptime(d1, "%Y-%m-%d", tz='UTC')], ans, warning="POSIXlt detected and converted to POSIXct")
 ll = list(a=as.POSIXlt("2015-01-01"), b=2L)
 test(1612.3, setDT(ll), error="Column 1 has class 'POSIXlt'")
 
@@ -18748,3 +18748,13 @@ test(2270, options=c(datatable.optimize=1L), DT[, mean(b, 1), by=a], data.table(
 DT1 = data.table(a=1:2)
 DT2 = data.table(a=c(1, 1, 2, 2), b=1:4)
 test(2271, options=c(datatable.verbose=TRUE), copy(DT1)[DT2, on='a', v := 4], copy(DT1)[, v := 4], output="x.a.\nAssigning")
+
+# convert POSIXlt to POSIXct in set and := #1724
+dt = data.table(a=1:2)
+dates_char = c("2024-01-01", "2024-01-02")
+dates = as.POSIXlt(dates_char, tz="UTC")
+ans = data.table(a=1:2, dates=as.POSIXct(dates))
+test(2272.1, copy(dt)[, dates := dates], ans, warning="POSIXlt.*converted to POSIXct")
+test(2272.2, set(copy(dt), j="dates", value=dates), ans, warning="POSIXlt.*converted to POSIXct")
+test(2272.3, {dt1 = copy(dt); dt1$dates=dates; dt1}, ans, warning="POSIXlt.*converted to POSIXct")
+test(2272.4, copy(dt)[, dates := strptime(dates_char, "%Y-%m-%d", tz="UTC")], ans, warning="POSIXlt.*converted to POSIXct")

src/assign.c

@@ -428,6 +428,10 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values)
   }
   if (any_duplicated(cols,FALSE)) error(_("Can't assign to the same column twice in the same query (duplicates detected)."));
   if (!isNull(newcolnames) && !isString(newcolnames)) error(_("newcolnames is supplied but isn't a character vector"));
+  if (Rf_inherits(values, "POSIXlt")) {
+    warning(_("Values of type POSIXlt detected and converted to POSIXct. We do not recommend the use of POSIXlt at all because it typically takes more than 6 times the storage as an equivalent POSIXct column. Use as.POSIXct() to avoid this warning."));
+    values = PROTECT(eval(PROTECT(lang2(sym_as_posixct, values)), R_GlobalEnv)); protecti+=2;
+  }
   bool RHS_list_of_columns = TYPEOF(values)==VECSXP && length(cols)>1;  // initial value; may be revised below
   if (verbose) Rprintf(_("RHS_list_of_columns == %s\n"), RHS_list_of_columns ? "true" : "false");
   if (TYPEOF(values)==VECSXP && length(cols)==1 && length(values)==1) {

src/data.table.h

@@ -107,6 +107,7 @@ extern SEXP sym_tzone;
 extern SEXP sym_old_fread_datetime_character;
 extern SEXP sym_variable_table;
 extern SEXP sym_as_character;
+extern SEXP sym_as_posixct;
 extern double NA_INT64_D;
 extern long long NA_INT64_LL;
 extern Rcomplex NA_CPLX;  // initialized in init.c; see there for comments

src/init.c

@@ -37,6 +37,7 @@ SEXP sym_tzone;
 SEXP sym_old_fread_datetime_character;
 SEXP sym_variable_table;
 SEXP sym_as_character;
+SEXP sym_as_posixct;
 double NA_INT64_D;
 long long NA_INT64_LL;
 Rcomplex NA_CPLX;
@@ -286,6 +287,7 @@ void attribute_visible R_init_data_table(DllInfo *info)
   sym_old_fread_datetime_character = install("datatable.old.fread.datetime.character");
   sym_variable_table = install("variable_table");
   sym_as_character = install("as.character");
+  sym_as_posixct = install("as.POSIXct");
 
   initDTthreads();
   avoid_openmp_hang_within_fork();