Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ S3method(seq, IDate)
S3method(split, IDate)
S3method(unique, IDate)
S3method(unique, ITime)
S3method('[<-', IDate)

# duplist
# getdots
Expand Down
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@

17. `example(":=", local=TRUE)` now works rather than error, [#2972](https://github.com/Rdatatable/data.table/issues/2972). Thanks @vlulla for the report.

18. `rbind.data.frame` on `IDate` columns changed the column from `integer` to `double`, [#2008](https://github.com/Rdatatable/data.table/issues/2008). Thanks to @rmcgehee for reporting.

#### NOTES

1. `rbindlist`'s `use.names="check"` now emits its message for automatic column names (`"V[0-9]+"`) too, [#3484](https://github.com/Rdatatable/data.table/pull/3484). See news item 5 of v1.12.2 below.
Expand Down
10 changes: 10 additions & 0 deletions R/IDateTime.R
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,16 @@ unique.IDate =
as.IDate(NextMethod())
}

# define this [<- method to prevent base R's internal rbind coercing integer IDate to double, #2008
`[<-.IDate` = function(x, i, value) {
if (!length(value)) return(x)
value = as.integer(as.IDate(value))
setattr(x, 'class', NULL)
x[i] = value
setattr(x, 'class', c('IDate', 'Date'))
x
}

# fix for #1315
as.list.IDate = function(x, ...) NextMethod()

Expand Down
16 changes: 8 additions & 8 deletions inst/tests/tests.Rraw
Original file line number Diff line number Diff line change
Expand Up @@ -993,16 +993,11 @@ test(343, DT[,(mynewcol):=21L], data.table(b=1:3,newname=21L))
mycols = 1:2
test(344, DT[,(mycols):=NULL], data.table(NULL))


# It seems that the .Internal rbind of two data.frame coerces IDate to numeric. Tried defining
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, the level of deja vu on reading this:

It seems that the .Internal rbind of two data.frame coerces IDate to numeric. Tried defining "[<-.IDate" as per Tom's suggestion, and c.IDate to no avail (maybe because the .Internal code in bind.c doesn't look up package methods?)

In fact I went through the same existential crisis for a while this morning! I'm not sure what was tried for [<-.IDate back then, maybe the NextMethod approach? I tried that as well but it seems fundamentally doomed because ... in NextMethod can't be passed as origin to the as.Date call in [<-.Date, which is needed if we pass the underlying integer to [<-.Date as value. If we pass an IDate, the #2008 problem happens -- double with IDate class. I didn't try running as.Date(value) but that would mean coercing numeric just to get [<-.Date and then back to integer aftewards... seems silly.

Anyway I think we can change this test that was testing that a bug was still a bug. Now the bug is fixed. I'm not sure if the subsequent test is still needed, but it can't hurt.

# "[<-.IDate" as per Tom's suggestion, and c.IDate to no avail (maybe because the .Internal code
# in bind.c doesn't look up package methods?). Anyway, as from 1.8.1, double are allowed in keys, so
# these still work but for a different reason than before 1.8.1: the results are IDate stored as double,
# rather than before when is worked because by and setkey coerced double to integer.
# Now, as from 1.12.4, the integer IDate in i is retained as integer rather than letting the double IDate persist.
# this originally tested that IDate would come back as double from rbind(DF, DF)
# Now #2008 is fixed in v1.12.4 it now checks it stays as an integer.
DF = data.frame(x=as.IDate(c("2010-01-01","2010-01-02")), y=1:6)
DT = as.data.table(rbind(DF,DF))
test(345, DT[,sum(y),by=x], {.x=as.IDate(c("2010-01-01","2010-01-02"));mode(.x)="double";data.table(x=.x,V1=c(18L,24L))})
test(345, DT[,sum(y),by=x], data.table(x=as.IDate(c("2010-01-01","2010-01-02")),V1=c(18L,24L)))
test(346, setkey(DT,x)[J(as.IDate("2010-01-02"))], data.table(x=as.IDate(rep("2010-01-02",6L)), y=rep(c(2L,4L,6L),2), key="x"))

# Test .N==0 with nomatch=NA|0, # tests for #963 added as well
Expand Down Expand Up @@ -14965,6 +14960,11 @@ test(2052, rbindlist(list(dt1, dt2), fill=TRUE),
data.table(V1=c(dt1$V1, dt2$V1),
V46=structure(c(3L,1L,1L,NA,3L,NA,NA,NA,NA,NA), .Label=c("Low","Typical","High",NA), class = c("ordered", "factor"))))

# rbind.data.frame didn't respect integer storage mode of IDate, #2008
DF = data.frame(date = as.IDate(0L))
test(2053.1, storage.mode(rbind(DF, DF)$date), 'integer')
test(2053.2, DF$date[1L] <- integer(), integer())


###################################
# Add new tests above this line #
Expand Down