From 35b834e88c65b64755e16ae1163a65fb32d6386c Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Sun, 18 Aug 2019 23:55:26 +0800 Subject: [PATCH] Closes #2068 -- improve strptime wrapper warning message --- NEWS.md | 2 ++ R/data.table.R | 2 +- inst/tests/tests.Rraw | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index bc6b599c35..8b44e9639b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -266,6 +266,8 @@ 15. `first` and `last` gain an explicit `n=1L` argument so that it's clear the default is 1, and their almost identical manual pages have been merged into one. +16. Re-wrote the warning about `strptime` usage in `j` to be more helpful/informative/actionable, [#2068](https://github.com/Rdatatable/data.table/issues/2068). Thanks to @tdhock for the report. + ### Changes in [v1.12.2](https://github.com/Rdatatable/data.table/milestone/14?closed=1) (07 Apr 2019) diff --git a/R/data.table.R b/R/data.table.R index 47c6729320..6dfdd272e5 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -1149,7 +1149,7 @@ replace_order = function(isub, verbose, env) { SDenv = new.env(parent=parent.frame()) # taking care of warnings for posixlt type, #646 SDenv$strptime = function(x, ...) { - warning("POSIXlt column type detected and converted to POSIXct. We do not recommend use of POSIXlt at all because it uses 40 bytes to store one date. Use as.POSIXct to avoid this warning.") + warning("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, ...)) } diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 58c3c63c4a..4a549ef7bc 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -8236,7 +8236,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="POSIXlt column type detected and converted") +test(1612.2, dt[, d2 := strptime(d1, "%Y-%m-%d", tz='UTC')], ans, warning="strptime() usage detected and wrapped with as.POSIXct()") ll = list(a=as.POSIXlt("2015-01-01"), b=2L) test(1612.3, setDT(ll), error="Column 1 is of POSIXlt type")