Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[R-Forge #1874] Add error message for unsupported column types such as POSIXlt. #646

Closed
arunsrinivasan opened this issue Jun 8, 2014 · 5 comments
Assignees
Milestone

Comments

@arunsrinivasan
Copy link
Member

Submitted by: Matt Dowle; Assigned to: Nobody; R-Forge link

In data.table(), as.data.table(), and := when creating new columns.
As reported here :
http://r.789695.n4.nabble.com/problems-with-adding-a-DateTime-column-tp4456838p4456838.html

@brodieG
Copy link

brodieG commented Aug 29, 2014

I would almost consider this a bug rather than a feature request. Maybe bug is too harsh a word, but it really surprised me that you could do this, especially given the surprising results you can get (also here).

Also, looks like you copied over the bug report from Source forge at #59 already (thanks!).

@tdhock
Copy link
Member

tdhock commented Nov 27, 2015

I confirm that this is still an issue on current master:

date.str <- "1984-03-17"
buggy.dt <- data.table(date.str)

## BUG: currently this warning is confusing (Supplied 11 items to be
## assigned to 2 items of column 'date.lt'). Instead I propose
## converting to a POSIXct object with a warning, as with
## data.table():
buggy.dt[, date.lt := strptime(date.str, "%Y-%m-%d")]
## Warning message:
## In data.table(date = strptime("2015-10-10", "%Y-%m-%d")) :
##   POSIXlt column type detected and converted to POSIXct. We do not recommend use of POSIXlt at all because it uses 40 bytes to store one date.
date.lt <- strptime(date.str, "%Y-%m-%d")

## I expected this should work as in base R:
(month.str <- strftime(date.lt, "%Y-%d-01"))
buggy.dt[, month.str := strftime(date.lt, "%Y-%d-01")]
## Error in as.POSIXlt.default(x, tz = tz) : 
##   do not know how to convert 'x' to class “POSIXlt”

## I also expected this conversion to work as in base R:
(date.ct <- as.POSIXct(date.lt))
buggy.dt[, date.ct := as.POSIXct(date.lt)]
## Error in as.POSIXct.default(date.lt) : 
##   do not know how to convert 'date.lt' to class “POSIXct”

Seems like the fix should be easy, just add a special case inside of [.data.table:

if(class of RHS of := is POSIXlt) LHS <- as.POSIXct(LHS)

However, after looking at the source code I am not sure where would be the most appropriate place to add this. If you can point me to where to add that, I could submit a PR.

@tdhock
Copy link
Member

tdhock commented Nov 27, 2015

btw

## The workaround is to use explicitly use as.POSIXct, but it would be
## nice if data.table did this for us automatically.
buggy.dt[, date.ct := as.POSIXct(strptime(date.str, "%Y-%m-%d"))]
buggy.dt[, month.str := strftime(date.ct, "%Y-%d-01")]

@jangorecki
Copy link
Member

@tdhock Warning about recycling is on point, POSIXlt is type of list, so potentially it could be consumed as such:

library(data.table)
x = as.POSIXlt(Sys.time())
typeof(x)
#[1] "list"
data.table(attr = names(unlist(x)))[, value := x][]
#      attr    value
# 1:    sec 31.91862
# 2:    min       27
# 3:   hour       15
# 4:   mday       27
# 5:    mon       10
# 6:   year      115
# 7:   wday        5
# 8:   yday      330
# 9:  isdst        0
# 10:   zone      GMT
# 11: gmtoff        0

Having a wrapper on strftime seems to be the best way to deal with it. You can always replace the original strftime function to not change the code at all.

strftime = function(...) as.POSIXct(base::strftime(...))

@arunsrinivasan arunsrinivasan self-assigned this Mar 4, 2016
@mattdowle
Copy link
Member

Minor fix 85bacc7 for Solaris, #1934.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants