fwrite: fix integer-backed POSIXct writing epoch instead of ISO #3535#7728
fwrite: fix integer-backed POSIXct writing epoch instead of ISO #3535#7728rmwood0908 wants to merge 2 commits intoRdatatable:masterfrom
Conversation
aitap
left a comment
There was a problem hiding this comment.
This is a good start, thank you.
| oldtz = Sys.getenv('TZ') | ||
| Sys.setenv(TZ = 'UTC') | ||
| tmp <- tempfile() | ||
| on.exit(Sys.setenv(TZ = oldtz), add = TRUE) |
There was a problem hiding this comment.
No need to adjust TZ, just give the tz = 'UTC' argument to as.POSIXct().
| fwrite(DT, tmp) | ||
| test(2368.1, capture.output(cat(readLines(tmp), sep = '\n')), c( |
There was a problem hiding this comment.
If you don't give an output argument to fwrite(), it prints to the standard output. You can then test the value of capture.output(fwrite(...)) without going through a temporary file (which you won't have to remember to unlink()).
data.table:::test also has an output argument, but those are skipped when running with a non-English locale, and this test must succeed no matter whether or not message translation is enabled.
| const void *tmp = &x; | ||
| writePOSIXct(tmp, 0, pch); |
There was a problem hiding this comment.
You can give &x straight to writePOSIXct() without going through a temporary variable. C allows this implicit conversion because the destination type is void *.
| @@ -23,6 +23,7 @@ writer_fun_t writeITime; | |||
| writer_fun_t writeDateInt32; | |||
| writer_fun_t writeDateFloat64; | |||
| writer_fun_t writePOSIXct; | |||
There was a problem hiding this comment.
What do you think about renaming this function to writePOSIXctDouble?
Closes #3535
Problem
Previously, the
fwrite()function would writePOSIXctdatetime columnsas epoch timestamps instead of the default ISO format when the column was
created with
seq():This happens because
seq()creates aPOSIXctcolumn backed by integers instead of doubles:Fix
Two changes were required:
src/fwriteR.c— Added aPOSIXctcheck so integer-backed datetime columns are routed to the correct writer instead of being treated as plain integers.src/fwrite.candsrc/fwrite.h— Added a newwritePOSIXctIntfunction that reads integer values, converts them to double, then formats them as datetime strings.Test Results
After applying the full fix, the output is now as desired
x
1970-01-01T00:00:00Z
1970-01-01T00:00:01Z
1970-01-01T00:00:02Z
1970-01-01T00:00:03Z
1970-01-01T00:00:04Z
1970-01-01T00:00:05Z
Test 2368.1 has been added to
inst/tests/tests.Rrawand a NEWS entry has been added as per contributing guidelines