From 8d2ed1f041a8132807e135e156ef67a7cb32e8d1 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Wed, 4 Dec 2019 15:01:50 +0800 Subject: [PATCH 1/2] default to tz="" for IDateTime POSIX methods --- NEWS.md | 2 ++ R/IDateTime.R | 4 ++-- inst/tests/tests.Rraw | 10 ++++++++++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index 4a0033fcfa..4140418b39 100644 --- a/NEWS.md +++ b/NEWS.md @@ -10,6 +10,8 @@ ## BUG FIXES +1. `as.IDate` and `as.ITime` methods for `POSIXct` input wrongly assigned `NULL` timezone to `UTC` (actually it's the session's default timezone), [#4085](https://github.com/Rdatatable/data.table/issues/4085). + ## NOTES 1. Links in the manual were creating warnings when installing HTML, [#4000](https://github.com/Rdatatable/data.table/issues/4000). Thanks to Morgan Jacob. diff --git a/R/IDateTime.R b/R/IDateTime.R index c719b20bb9..265d57ffb3 100644 --- a/R/IDateTime.R +++ b/R/IDateTime.R @@ -33,7 +33,7 @@ as.IDate.Date = function(x, ...) { } as.IDate.POSIXct = function(x, tz = attr(x, "tzone", exact=TRUE), ...) { - if (is.null(tz)) tz = "UTC" + if (is.null(tz)) tz='' if (tz %chin% c("UTC", "GMT")) { (setattr(as.integer(as.numeric(x) %/% 86400L), "class", c("IDate", "Date"))) # %/% returns new object so can use setattr() on it; wrap with () to return visibly } else @@ -138,7 +138,7 @@ as.ITime.default = function(x, ...) { } as.ITime.POSIXct = function(x, tz = attr(x, "tzone", exact=TRUE), ...) { - if (is.null(tz)) tz = "UTC" + if (is.null(tz)) tz='' if (tz %chin% c("UTC", "GMT")) as.ITime(unclass(x), ...) else as.ITime(as.POSIXlt(x, tz = tz, ...), ...) } diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index f10fe26ce1..dedd5d551f 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -16381,6 +16381,16 @@ test(2121.5, DT[, .(.N=.N), by=a], data.table(a=c(1,2), .N=2:1)) # user supplied ## { ending in NULL should retain that NULL, #4061 test(2121.6, DT[ , {.(a, b=b+1); NULL}], NULL) +# system timezone is not usually UTC, so as.ITime.POSIXct shouldn't assume so +oldtz=Sys.getenv('TZ') +Sys.setenv(TZ='Asia/Jakarta') # UTC+7 +t0 = Sys.time() +test(2122.1, as.integer(unclass(as.ITime(t0)) %/% 3600), as.POSIXlt(t0)$hour) +d0 = as.POSIXct(trunc(t0, 'day')) +# base defaults +test(2122.2, unclass(as.IDate(d0)), as.integer(as.Date(d0, tz=''))) +Sys.setenv(TZ=oldtz) + ################################### # Add new tests above this line # From 374fa8d3368e36b0526b24e89ba200ef673d6636 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Wed, 4 Dec 2019 15:05:45 +0800 Subject: [PATCH 2/2] simplify tests --- inst/tests/tests.Rraw | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index dedd5d551f..6660052f02 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -16384,11 +16384,9 @@ test(2121.6, DT[ , {.(a, b=b+1); NULL}], NULL) # system timezone is not usually UTC, so as.ITime.POSIXct shouldn't assume so oldtz=Sys.getenv('TZ') Sys.setenv(TZ='Asia/Jakarta') # UTC+7 -t0 = Sys.time() -test(2122.1, as.integer(unclass(as.ITime(t0)) %/% 3600), as.POSIXlt(t0)$hour) -d0 = as.POSIXct(trunc(t0, 'day')) -# base defaults -test(2122.2, unclass(as.IDate(d0)), as.integer(as.Date(d0, tz=''))) +t0 = as.POSIXct('2019-10-01') +test(2122.1, format(as.ITime(t0)), '00:00:00') +test(2122.2, format(as.IDate(t0)), '2019-10-01') Sys.setenv(TZ=oldtz)