Repository navigation
Fwrite should always quote empty strings - #2216
Conversation
…), so that NA strings and empty strings can be distinguished in the output. Closes #2215
|
Seems like something broke in package |
|
@st-pasha it seems to be related to nanotime upgrade on CRAN on 22/06. |
|
Yes we make substantial changes to nanotime since the very, very beginning when this first example was set up. I am not entirely sure why I think we may have been going through int64 before but don't think we need that anymore. Now, what nanotime itself does relies on RcppCCTZ where data.table has its own in writeNanotime. I suspect it may need a closer look. @mattdowle can you take a look there? |
|
I have an idea. We moved the representation from S3 to S4, so the test for 'is it nanotime?' in R> library(data.table)
data.table 1.10.5 IN DEVELOPMENT built 2017-06-25 12:14:47.993013 UTC
The fastest way to learn (by data.table authors): https://www.datacamp.com/courses/data-analysis-the-data-table-way
Documentation: ?data.table, example(data.table) and browseVignettes("data.table")
Release notes, videos and slides: http://r-datatable.com
R> library(nanotime)
R> v <- nanotime(Sys.time())+1:5
R> dt <- data.table(cbind(a=1:5, b=5:1), v)
R> dt
a b v
1: 1 5 2017-06-25T12:19:30.015459001+00:00
2: 2 4 2017-06-25T12:19:30.015459002+00:00
3: 3 3 2017-06-25T12:19:30.015459003+00:00
4: 4 2 2017-06-25T12:19:30.015459004+00:00
5: 5 1 2017-06-25T12:19:30.015459005+00:00
R> fwrite(dt, "/tmp/newdt.csv")
Write integer
Write integer
Write integer
Write integer
Write integer
Write integer
Write integer
Write integer
Write integer
Write integer
Write integer
Write integer
Write integer
Write integer
Write integer
Write integer
Write integer
Write integer
Write integer
Write integer
R>
|
|
Here's a small patch that works for @eddelbuettel's example. I didn't want to remove code, so I left the return for diff --git a/src/fwrite.c b/src/fwrite.c
index f49a24a..55727f7 100644
--- a/src/fwrite.c
+++ b/src/fwrite.c
@@ -496,6 +496,10 @@ static writer_fun_t whichWriter(SEXP column) {
case REALSXP:
if (INHERITS(column, char_integer64))
return (INHERITS(column, char_nanotime) && dateTimeAs!=DATETIMEAS_EPOCH) ? writeNanotime : writeInteger;
+ if (IS_S4_OBJECT(column)) {
+ SEXP class = STRING_ELT(getAttrib(column, R_ClassSymbol), 0);
+ return (class == char_nanotime) ? writeNanotime : writeInteger;
+ }
if (dateTimeAs==DATETIMEAS_EPOCH) return writeNumeric;
if (INHERITS(column, char_Date)) return writeDateReal;
if (INHERITS(column, char_POSIXct)) return writePOSIXct;R> library(data.table)
R> library(nanotime)
R> v <- nanotime(Sys.time())+1:5
R> dt <- data.table(cbind(a=1:5, b=5:1), v)
R> dt
a b v
1: 1 5 2017-06-25T15:23:08.303934001+00:00
2: 2 4 2017-06-25T15:23:08.303934002+00:00
3: 3 3 2017-06-25T15:23:08.303934003+00:00
4: 4 2 2017-06-25T15:23:08.303934004+00:00
5: 5 1 2017-06-25T15:23:08.303934005+00:00
R> fwrite(dt, "/tmp/newdt.csv")
R> readLines("/tmp/newdt.csv")
[1] "a,b,v" "1,5,2017-06-25T15:23:08.303934001Z"
[3] "2,4,2017-06-25T15:23:08.303934002Z" "3,3,2017-06-25T15:23:08.303934003Z"
[5] "4,2,2017-06-25T15:23:08.303934004Z" "5,1,2017-06-25T15:23:08.303934005Z" |
|
@eddelbuettel Thanks for the investigation. I can make the change. But it is failing on CRAN. Did you check reverse dependencies before releasing nanotime to CRAN? |
…N 22 June 2017. Discussion in #2216
|
@mattdowle Sorry, I did not. We changed from S3 to S4, invalidating your existing test. But the suggested change by @joshuaulrich should address this. |
Codecov Report
@@ Coverage Diff @@
## master #2216 +/- ##
==========================================
+ Coverage 90.69% 90.69% +<.01%
==========================================
Files 59 59
Lines 11476 11481 +5
==========================================
+ Hits 10408 10413 +5
Misses 1068 1068
Continue to review full report at Codecov.
|
6 participants
(under quote='auto' at least), so that NA strings and empty strings can be distinguished in the output. Closes #2215