Skip to content

Conversation

@fparages
Copy link
Member

@fparages fparages commented Apr 10, 2019

Allows printing of timezones when printing data.tables with POSIX columns (if tzone attribute is defined).

Closes #2842. An item in #1523 list.

Defined as optional via a parameter timezone with default value = FALSE, for backwards compatibility.

@fparages
Copy link
Member Author

In line with @franknarf1 's comment in #1523 :
What's the consensus: would you prefer the new feature to print out time offset with respect to UTC, or the full tz name (as it is PRed now)?

@MichaelChirico
Copy link
Member

MichaelChirico commented Apr 13, 2019

https://en.wikipedia.org/wiki/ISO_8601

ISO 8601 timestamps would be ideal IMO:

Time zones in ISO 8601 are represented as local time (with the location unspecified), as UTC, or as an offset from UTC.

Many other languages (e.g. Spark, Presto, Go) also understand the T delimiter between YMD and HMS... I also think base R's strptime should be improved to handle ISO 8601 timestamps natively but that's another issue...

It would be amazing if fwrite could write ISO timestamps & fread could read them but that feels a bit like scope creep -- I think the improvement to upstream in base R would be better...

@mattdowle
Copy link
Member

Thanks @fparages! I've invited you to be project member. (You'll need to accept the invite before GitHub adds you.)

@mattdowle mattdowle added this to the 1.12.4 milestone Apr 15, 2019
@codecov
Copy link

codecov bot commented Apr 15, 2019

Codecov Report

Merging #3500 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3500      +/-   ##
==========================================
+ Coverage   96.66%   96.66%   +<.01%     
==========================================
  Files          65       65              
  Lines       12278    12288      +10     
==========================================
+ Hits        11868    11878      +10     
  Misses        410      410
Impacted Files Coverage Δ
R/print.data.table.R 93.39% <100%> (+0.68%) ⬆️

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 96543dd...6baba28. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 15, 2019

Codecov Report

Merging #3500 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3500      +/-   ##
==========================================
+ Coverage   96.66%   96.66%   +<.01%     
==========================================
  Files          65       65              
  Lines       12278    12288      +10     
==========================================
+ Hits        11868    11878      +10     
  Misses        410      410
Impacted Files Coverage Δ
R/print.data.table.R 93.39% <100%> (+0.68%) ⬆️

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 96543dd...942f2ba. Read the comment docs.

@mattdowle mattdowle merged commit 7a570bb into Rdatatable:master Apr 15, 2019
MichaelChirico added a commit that referenced this pull request May 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

printing of POSIXct should (could?) include tz info if it's there

3 participants