need to call as.POSIXct() with tz after "TZ" ENV VAR gets changed by shrektan · Pull Request #4261 · Rdatatable/data.table

shrektan · 2020-02-27T10:38:15Z

Otherwise, it will fail on macOS. See #4261 (comment) for the details.

Update: I've reported this on https://bugs.r-project.org/bugzilla/show_bug.cgi?id=17724

Update: As Luke Tierney points out (in the above URL), unset the env var by calling Sys.setenv(TZ="") is wrong. The correct way is to call Sys.unsetenv("TZ").

otherwise it will fail if the locale timezone is not UTC

codecov · 2020-02-27T10:58:23Z

Codecov Report

Merging #4261 (12d8bd6) into master (336187b) will not change coverage.
The diff coverage is n/a.

❗ Current head 12d8bd6 differs from pull request most recent head dcfb0d6. Consider uploading reports for the commit dcfb0d6 to get more accurate results

@@           Coverage Diff           @@
##           master    #4261   +/-   ##
=======================================
  Coverage   99.46%   99.46%           
=======================================
  Files          75       75           
  Lines       14712    14712           
=======================================
  Hits        14634    14634           
  Misses         78       78

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 336187b...dcfb0d6. Read the comment docs.

MichaelChirico · 2020-02-27T11:21:06Z

@jangorecki can we add TZ=Asia/Jakarta (or any non-UTC tz) to one of the GL CI test suites?

Maybe also something LANGUAGE=es_AR to check sensitivity to locale as well

jangorecki · 2020-02-27T11:32:41Z

@MichaelChirico windows builds runs on

TZ=America/Los_Angeles, locale='LC_COLLATE=C;LC_CTYPE=English_United States.1252;LC_MONETARY=English_United States.1252;LC_NUMERIC=C;LC_TIME=English_United States.1252'

https://gitlab.com/Rdatatable/data.table/-/jobs/443985937/artifacts/file/bus/test-rel-win/data.table.Rcheck/tests_x64/main.Rout
there are already tests 2124 related to tz

MichaelChirico · 2020-02-27T16:06:19Z

@jangorecki I see, good to know. Is that a property of the docker image? I don't see anything in .gitlab-ci.yml about it

In any case, it appears we missed this issue anyway -- would it make sense to add a different TZ & locale to one of the other builds?

@shrektan what's your Sys.timezone() / locale info? Also which tests are failing exactly?

shrektan · 2020-02-27T17:38:47Z

Sys.timezone()

[1] "Asia/Shanghai"

error related

Error in test.data.table() : 
  6 errors out of 9867. Search tests/tests.Rraw for test numbers: 2137.04, 2137.06, 2137.07, 2137.08, 2137.09, 2137.1.

Code that below comment uses

library(data.table)
DT = data.table(hour31   = as.ITime(seq(as.POSIXct("2020-01-01 07:00:40"), by = "31 min", length.out = 9)),
                hour30   = as.ITime(seq(as.POSIXct("2020-01-01 07:00:00"), by = "30 min", length.out = 9)),
                minute31 = as.ITime(seq(as.POSIXct("2020-01-01 07:00:00"), by = "31 sec", length.out = 9)),
                minute30 = as.ITime(seq(as.POSIXct("2020-01-01 07:00:00"), by = "30 sec", length.out = 9)))
DT

An interesting observation is:

Before I run test.data.table(), the above code returns:

     hour31   hour30 minute31 minute30
1: 07:00:40 07:00:00 07:00:00 07:00:00
2: 07:31:40 07:30:00 07:00:31 07:00:30
3: 08:02:40 08:00:00 07:01:02 07:01:00
4: 08:33:40 08:30:00 07:01:33 07:01:30
5: 09:04:40 09:00:00 07:02:04 07:02:00
6: 09:35:40 09:30:00 07:02:35 07:02:30
7: 10:06:40 10:00:00 07:03:06 07:03:00
8: 10:37:40 10:30:00 07:03:37 07:03:30
9: 11:08:40 11:00:00 07:04:08 07:04:00

After test.data.table(), the result becomes:

     hour31   hour30 minute31 minute30
    <ITime>  <ITime>  <ITime>  <ITime>
1: 15:00:40 15:00:00 15:00:00 15:00:00
2: 15:31:40 15:30:00 15:00:31 15:00:30
3: 16:02:40 16:00:00 15:01:02 15:01:00
4: 16:33:40 16:30:00 15:01:33 15:01:30
5: 17:04:40 17:00:00 15:02:04 15:02:00
6: 17:35:40 17:30:00 15:02:35 15:02:30
7: 18:06:40 18:00:00 15:03:06 15:03:00
8: 18:37:40 18:30:00 15:03:37 15:03:30
9: 19:08:40 19:00:00 15:04:08 15:04:00

What's more interesting is:

If I add a browser() into the test.Rraw file, the execution will stop in the middle, and if I choose to continue, the error will not occur. However, if I remove the browser() line, the error will occur.

Again, I checked both Sys.getlocale() and Sys.timezone(), they returned the same, as if never changed.

shrektan · 2020-02-27T17:45:09Z

See this weird issue...

Before it returns expected

d> as.POSIXct("2020-01-01 07:00:40") 
[1] "2020-01-01 07:00:40 CST"

After I run `test.data.table()` error throws and the same code returns differently

jangorecki · 2020-02-28T03:31:14Z

@MichaelChirico no, windows builds runs not in a docker but using ssh executor on windows OS directly.

shrektan · 2020-02-28T12:03:58Z

OK I just find out the root causes:

data.table/inst/tests/tests.Rraw

Lines 16554 to 16560 in b1b1832

    
           # system timezone is not usually UTC, so as.ITime.POSIXct shouldn't assume so, #4085 
        
           oldtz=Sys.getenv('TZ') 
        
           Sys.setenv(TZ='Asia/Jakarta') # UTC+7 
        
           t0 = as.POSIXct('2019-10-01') 
        
           test(2124.1, format(as.ITime(t0)), '00:00:00') 
        
           test(2124.2, format(as.IDate(t0)), '2019-10-01') 
        
           Sys.setenv(TZ=oldtz)

I'm not sure it's a bug of R or not but we just need to call as.POSIXct with tz explicitly after we change the TZ env variable.

In addition, it may happen on Mac Only because I can't reproduce it on Windows and since the CI works fine, I doubt it happens on Linux - but I would happy if somebody can help me to verify this.

# system timezone is not usually UTC, so as.ITime.POSIXct shouldn't assume so, #4085
as.POSIXct("2020-01-01 07:00:40")
#> [1] "2020-01-01 07:00:40 CST"
oldtz=Sys.getenv('TZ')
Sys.setenv(TZ='Asia/Jakarta') # UTC+7
as.POSIXct("2020-01-01 07:00:40")
#> [1] "2020-01-01 07:00:40 WIB"
Sys.setenv(TZ=oldtz)
####### this is unexpected ##########
as.POSIXct("2020-01-01 07:00:40")
#> [1] "2020-01-01 15:00:40 CST"
####### after call as.POSIXct() with tz explicitly ###########
as.POSIXct("2020-01-01 07:00:40", tz = Sys.timezone())
#> [1] "2020-01-01 07:00:40 CST"
####### it falls back to normal ###########
as.POSIXct("2020-01-01 07:00:40")
#> [1] "2020-01-01 07:00:40 CST"

^{Created on 2020-02-28 by the reprex package (v0.3.0)}

sessioninfo

> sessionInfo()
R version 3.6.2 (2019-12-12)
Platform: x86_64-apple-darwin15.6.0 (64-bit)
Running under: macOS Catalina 10.15.3

Matrix products: default
BLAS:   /System/Library/Frameworks/Accelerate.framework/Versions/A/Frameworks/vecLib.framework/Versions/A/libBLAS.dylib
LAPACK: /Library/Frameworks/R.framework/Versions/3.6/Resources/lib/libRlapack.dylib

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] data.table_1.12.9 usethis_1.5.1    

loaded via a namespace (and not attached):
 [1] Rcpp_1.0.3           rstudioapi_0.10      whisker_0.4          knitr_1.27.2        
 [5] GCAMCPUB_0.3.17      magrittr_1.5         bit_1.1-15.1         core_0.1.7          
 [9] lattice_0.20-38      R6_2.4.1             rlang_0.4.4          stringr_1.4.0       
[13] tools_3.6.2          xts_0.12-0           grid_3.6.2           xfun_0.12           
[17] DBI_1.1.0            clipr_0.7.0          htmltools_0.4.0.9002 bit64_0.9-7         
[21] yaml_2.2.1           digest_0.6.23        assertthat_0.2.1     processx_3.4.1      
[25] callr_3.4.1          purrr_0.3.3          ps_1.3.0             microbenchmark_1.4-7
[29] fs_1.3.1             glue_1.3.1           evaluate_0.14        rmarkdown_2.1       
[33] reprex_0.3.0         stringi_1.4.5        compiler_3.6.2       lubridate_1.7.4     
[37] zoo_1.8-7

This reverts commit 0933bae.

… on macOS due to a possible bug of R's mac binary

MichaelChirico · 2020-03-01T11:06:11Z

on rocker/r-base I see:

Sys.timezone()
# [1] "Etc/UTC"
as.POSIXct("2020-01-01 07:00:40")
# [1] "2020-01-01 07:00:40 UTC"
oldtz=Sys.getenv('TZ')
Sys.setenv(TZ='Asia/Jakarta') # UTC+7
as.POSIXct("2020-01-01 07:00:40")
# [1] "2020-01-01 07:00:40 WIB"
Sys.setenv(TZ=oldtz)
as.POSIXct("2020-01-01 07:00:40")
# [1] "2020-01-01 07:00:40 UTC"
as.POSIXct("2020-01-01 07:00:40", tz = Sys.timezone())
# [1] "2020-01-01 07:00:40 UTC"
as.POSIXct("2020-01-01 07:00:40")
# [1] "2020-01-01 07:00:40 UTC"

On my Mac I see:

Sys.timezone()
# [1] "Asia/Kuala_Lumpur"
as.POSIXct("2020-01-01 07:00:40")
# [1] "2020-01-01 07:00:40 +08"
oldtz=Sys.getenv('TZ')
Sys.setenv(TZ='Asia/Jakarta') # UTC+7
as.POSIXct("2020-01-01 07:00:40")
# [1] "2020-01-01 07:00:40 WIB"
Sys.setenv(TZ=oldtz)
as.POSIXct("2020-01-01 07:00:40")
# [1] "2020-01-01 15:00:40 +08"
as.POSIXct("2020-01-01 07:00:40", tz = Sys.timezone())
# [1] "2020-01-01 07:00:40 +08"
as.POSIXct("2020-01-01 07:00:40")
# [1] "2020-01-01 07:00:40 +08"

…ill fail on macOS due to a possible bug of R's mac binary" This reverts commit 9ddf2a7.

unset and empty env var are different and it leads to unexpected results if env var "TZ" is set to "" on macOS

MichaelChirico · 2020-03-01T17:04:15Z


 # system timezone is not usually UTC, so as.ITime.POSIXct shouldn't assume so, #4085
-oldtz=Sys.getenv('TZ')
+oldtz = Sys.getenv('TZ', unset = NA)


nice catch, h/t to Luke

mattdowle · 2021-05-13T23:48:30Z

Linking to #4088 and #4085 might may have been involved.
It seems that the changes in this PR are already in master from #4464.
This PR becomes an additional comment then to link it up.

should set tz="UTC" explicitly in test 2137

0933bae

otherwise it will fail if the locale timezone is not UTC

shrektan changed the title ~~should set tz="UTC" explicitly in test 2137~~ need to call as.POSIXct() with tz after "TZ" ENV VAR gets changed Feb 28, 2020

shrektan added 2 commits February 28, 2020 20:16

Revert "should set tz="UTC" explicitly in test 2137"

78f6293

This reverts commit 0933bae.

need to call as.POSIXct() with tz explicitly otherwise it will fail…

9ddf2a7

… on macOS due to a possible bug of R's mac binary

shrektan force-pushed the fixtest2137 branch from 28dea3a to 9ddf2a7 Compare February 28, 2020 12:17

shrektan added 2 commits March 2, 2020 00:40

Revert "need to call as.POSIXct() with tz explicitly otherwise it w…

0eba2f8

…ill fail on macOS due to a possible bug of R's mac binary" This reverts commit 9ddf2a7.

should call Sys.unsetenv() when we want to "unset" the env var

3841741

unset and empty env var are different and it leads to unexpected results if env var "TZ" is set to "" on macOS

MichaelChirico reviewed Mar 1, 2020

View reviewed changes

MichaelChirico mentioned this pull request May 19, 2020

[BUGZILLA #17724] as.POSIXct returns incorrect results if env "TZ" is touched on macOS MichaelChirico/r-bugs#6898

Open

mattdowle added this to the 1.14.1 milestone May 13, 2021

mattdowle added 2 commits May 13, 2021 17:33

Merge branch 'master' into fixtest2137

7e3ae57

revert whitespace

2a219d2

mattdowle removed this from the 1.14.1 milestone May 13, 2021

link comment to PRs

dcfb0d6

mattdowle added this to the 1.14.1 milestone May 14, 2021

mattdowle merged commit 2d88099 into master May 14, 2021

mattdowle deleted the fixtest2137 branch May 14, 2021 00:44

jangorecki modified the milestones: 1.14.9, 1.15.0 Oct 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

need to call as.POSIXct() with tz after "TZ" ENV VAR gets changed#4261

need to call as.POSIXct() with tz after "TZ" ENV VAR gets changed#4261
mattdowle merged 8 commits intomasterfrom
fixtest2137

shrektan commented Feb 27, 2020 •

edited

Loading

Uh oh!

codecov bot commented Feb 27, 2020 •

edited

Loading

Uh oh!

MichaelChirico commented Feb 27, 2020

Uh oh!

jangorecki commented Feb 27, 2020 •

edited

Loading

Uh oh!

MichaelChirico commented Feb 27, 2020 •

edited

Loading

Uh oh!

shrektan commented Feb 27, 2020

Uh oh!

shrektan commented Feb 27, 2020

Uh oh!

jangorecki commented Feb 28, 2020

Uh oh!

shrektan commented Feb 28, 2020 •

edited

Loading

Uh oh!

MichaelChirico commented Mar 1, 2020 •

edited

Loading

Uh oh!

MichaelChirico Mar 1, 2020

Uh oh!

mattdowle commented May 13, 2021 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

shrektan commented Feb 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

MichaelChirico commented Feb 27, 2020

Uh oh!

jangorecki commented Feb 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MichaelChirico commented Feb 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shrektan commented Feb 27, 2020

Sys.timezone()

error related

Code that below comment uses

An interesting observation is:

What's more interesting is:

Uh oh!

shrektan commented Feb 27, 2020

Before it returns expected

After I run test.data.table() error throws and the same code returns differently

Uh oh!

jangorecki commented Feb 28, 2020

Uh oh!

shrektan commented Feb 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MichaelChirico commented Mar 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MichaelChirico Mar 1, 2020

Choose a reason for hiding this comment

Uh oh!

mattdowle commented May 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

shrektan commented Feb 27, 2020 •

edited

Loading

codecov bot commented Feb 27, 2020 •

edited

Loading

jangorecki commented Feb 27, 2020 •

edited

Loading

MichaelChirico commented Feb 27, 2020 •

edited

Loading

After I run `test.data.table()` error throws and the same code returns differently

shrektan commented Feb 28, 2020 •

edited

Loading

MichaelChirico commented Mar 1, 2020 •

edited

Loading

mattdowle commented May 13, 2021 •

edited

Loading