-
Notifications
You must be signed in to change notification settings - Fork 1k
Preserve names in as.IDate() and as.ITime()
#7800
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
Changes from all commits
32d2049
1d53123
197dfd7
1905e89
d57b40a
d962514
69b33aa
c28278f
8b3ec4d
72a9e31
ab4eeea
7a1dc22
d19dbb2
38a9439
598d387
342b567
90da5da
b40e180
113f35b
f3e235d
a4761fa
ede10ca
2e73c76
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,11 @@ | |
|
|
||
| as.IDate = function(x, ...) UseMethod("as.IDate") | ||
|
|
||
| copy_names = function(ans, nm) { | ||
| if (!is.null(nm)) setattr(ans, "names", nm) | ||
| ans | ||
| } | ||
|
|
||
| as.IDate.default = function(x, ..., tz = attr(x, "tzone", exact=TRUE)) { | ||
| if (is.null(tz)) tz = "UTC" | ||
| if (is.character(x)) { | ||
|
|
@@ -17,30 +22,35 @@ as.IDate.default = function(x, ..., tz = attr(x, "tzone", exact=TRUE)) { | |
| as.IDate.numeric = function(x, origin = "1970-01-01", ...) { | ||
| if (origin=="1970-01-01") { | ||
| # standard epoch | ||
| nm = names(x) | ||
| x = as.integer(x) | ||
| class(x) = c("IDate", "Date") | ||
| # We used to use structure() here because class(x)<- copied several times in R before v3.1.0 | ||
| # Since R 3.1.0 improved class()<- and data.table's oldest oldest supported R is now >3.1.0, we can use class<- again | ||
| # structure() contains a match() and replace for specials, which we don't need. | ||
| # class()<- ensures at least 1 shallow copy as appropriate is returned. | ||
| x | ||
| copy_names(x, nm) | ||
| } else { | ||
| # only call expensive as.IDate.character if we have to | ||
| as.IDate(origin, ...) + as.integer(x) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We cannot drop the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please also add this as regression test
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds okay to me |
||
| as.IDate(origin, ...) + copy_names(as.integer(x), names(x)) | ||
|
ben-schwen marked this conversation as resolved.
|
||
| } | ||
| } | ||
|
|
||
| as.IDate.Date = function(x, ...) { | ||
| nm = names(x) | ||
| x = as.integer(x) # if already integer, x will be left unchanged as the original input | ||
| class(x) = c("IDate", "Date") # class()<- will copy if as.integer() did not create, and may not if it did we hope | ||
| x # always return a new object | ||
| copy_names(x, nm) | ||
| } | ||
|
|
||
| as.IDate.POSIXct = function(x, tz = attr(x, "tzone", exact=TRUE), ...) { | ||
| if (is_utc(tz)) | ||
| (setattr(as.integer(as.numeric(x) %/% 86400L), "class", c("IDate", "Date"))) # %/% returns new object so can use setattr() on it; wrap with () to return visibly | ||
| else | ||
| if (is_utc(tz)) { | ||
| ans = as.integer(as.numeric(x) %/% 86400L) # %/% returns a new object, so setattr() is safe | ||
| setattr(ans, "class", c("IDate", "Date")) | ||
| copy_names(ans, names(x)) | ||
| } else { | ||
| as.IDate(as.Date(x, tz = tz %||% '', ...)) | ||
| } | ||
| } | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we remove the blank line here? |
||
| as.IDate.IDate = function(x, ...) x | ||
|
|
@@ -112,7 +122,11 @@ chooseOpsMethod.IDate = function(x, y, mx, my, cl, reverse) inherits(y, "Date") | |
| } | ||
| if (inherits(e1, "Date") && inherits(e2, "Date")) | ||
| stopf("binary + is not defined for \"IDate\" objects") | ||
| (setattr(as.integer(unclass(e1) + unclass(e2)), "class", c("IDate", "Date"))) # () wrap to return visibly | ||
| res = unclass(e1) + unclass(e2) | ||
| nm = names(res) | ||
| ans = as.integer(res) | ||
| setattr(ans, "class", c("IDate", "Date")) | ||
| copy_names(ans, nm) | ||
| } | ||
|
|
||
| `-.IDate` = function(e1, e2) { | ||
|
|
@@ -139,14 +153,16 @@ chooseOpsMethod.IDate = function(x, y, mx, my, cl, reverse) inherits(y, "Date") | |
| return(base::`-.Date`(as.Date(e1), e2)) | ||
| # can't call base::.Date directly (last line of base::`-.Date`) as tried in PR#3168 because ?.Date states "Internal objects in the base package most of which are only user-visible because of the special nature of the base namespace." | ||
| } | ||
| ans = as.integer(unclass(e1) - unclass(e2)) | ||
| res = unclass(e1) - unclass(e2) | ||
| nm = names(res) | ||
| ans = as.integer(res) | ||
| if (inherits(e2, "Date")) { | ||
| setattr(ans, "class", "difftime") | ||
| setattr(ans, "units", "days") | ||
| } else { | ||
| setattr(ans, "class", c("IDate", "Date")) | ||
| } | ||
| ans | ||
| copy_names(ans, nm) | ||
| } | ||
|
|
||
|
|
||
|
|
@@ -168,13 +184,18 @@ as.ITime.POSIXct = function(x, tz = attr(x, "tzone", exact=TRUE), ...) { | |
| } | ||
|
|
||
| as.ITime.numeric = function(x, ms = 'truncate', ...) { | ||
| nm = names(x) | ||
| secs = clip_msec(x, ms) %% 86400L # the %% here ensures a local copy is obtained; the truncate as.integer() may not copy | ||
| (setattr(secs, "class", "ITime")) | ||
| setattr(secs, "class", "ITime") | ||
| copy_names(secs, nm) | ||
| } | ||
|
|
||
| as.ITime.character = function(x, format, ...) { | ||
| nm = names(x) | ||
| x = unclass(x) | ||
| if (!missing(format)) return(as.ITime(strptime(x, format = format, ...), ...)) | ||
| if (!missing(format)) { | ||
| return(copy_names(as.ITime(strptime(x, format = format, ...), ...), nm)) | ||
| } | ||
| # else allow for mixed formats, such as test 1189 where seconds are caught despite varying format | ||
| y = strptime(x, format = "%H:%M:%OS", ...) | ||
| w = which(is.na(y)) | ||
|
|
@@ -194,18 +215,23 @@ as.ITime.character = function(x, format, ...) { | |
| w = w[!nna] | ||
| } | ||
| } | ||
| as.ITime(y, ...) | ||
| copy_names(as.ITime(y, ...), nm) | ||
| } | ||
|
|
||
| as.ITime.POSIXlt = function(x, ms = 'truncate', ...) { | ||
| nm = names(x) | ||
| secs = clip_msec(x$sec, ms) | ||
| (setattr(with(x, secs + min * 60L + hour * 3600L), "class", "ITime")) # () wrap to return visibly | ||
| ans = with(x, secs + min * 60L + hour * 3600L) | ||
| setattr(ans, "class", "ITime") | ||
| copy_names(ans, nm) | ||
| } | ||
|
|
||
| as.ITime.times = function(x, ms = 'truncate', ...) { | ||
| nm = names(x) | ||
| secs = 86400L * (unclass(x) %% 1L) | ||
| secs = clip_msec(secs, ms) | ||
| (setattr(secs, "class", "ITime")) # the first line that creates sec will create a local copy so we can use setattr() to avoid potential copy of class()<- | ||
| setattr(secs, "class", "ITime") # the first line that creates sec will create a local copy so we can use setattr() to avoid potential copy of class()<- | ||
| copy_names(secs, nm) | ||
| } | ||
|
|
||
| as.character.ITime = format.ITime = function(x, ...) { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.