You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/03/15 14:30:28 UTC
[GitHub] [arrow] jonkeane commented on a change in pull request #12506: ARROW-15098 [R] Add binding for lubridate::duration() and/or as.difftime()
jonkeane commented on a change in pull request #12506:
URL: https://github.com/apache/arrow/pull/12506#discussion_r827026761
##########
File path: r/R/dplyr-funcs-datetime.R
##########
@@ -189,6 +189,70 @@ register_bindings_datetime <- function() {
})
}
+register_bindings_duration <- function() {
+ register_binding("difftime", function(time1,
+ time2,
+ tz,
+ units = c("auto", "secs", "mins",
+ "hours", "days", "weeks")) {
+ units <- match.arg(units)
+ if (units != "secs") {
+ abort("`difftime()` with units other than seconds not supported in Arrow")
+ }
+
+ # for time32() we do not need to worry about timezone
Review comment:
Is this stale?
##########
File path: r/R/dplyr-funcs-datetime.R
##########
@@ -189,6 +189,70 @@ register_bindings_datetime <- function() {
})
}
+register_bindings_duration <- function() {
+ register_binding("difftime", function(time1,
+ time2,
+ tz,
+ units = c("auto", "secs", "mins",
+ "hours", "days", "weeks")) {
+ units <- match.arg(units)
+ if (units != "secs") {
+ abort("`difftime()` with units other than seconds not supported in Arrow")
+ }
+
+ # for time32() we do not need to worry about timezone
+ if (call_binding("is.instant", time1) & call_binding("is.instant", time2)) {
Review comment:
we might want look at where is.instant is defined and choose a different one?
##########
File path: r/R/dplyr-funcs-datetime.R
##########
@@ -189,6 +189,70 @@ register_bindings_datetime <- function() {
})
}
+register_bindings_duration <- function() {
+ register_binding("difftime", function(time1,
+ time2,
+ tz,
+ units = c("auto", "secs", "mins",
+ "hours", "days", "weeks")) {
+ units <- match.arg(units)
+ if (units != "secs") {
+ abort("`difftime()` with units other than seconds not supported in Arrow")
+ }
+
+ # for time32() we do not need to worry about timezone
+ if (call_binding("is.instant", time1) & call_binding("is.instant", time2)) {
+ if (missing(tz)) {
+ time1 <- build_expr("cast", time1, options = cast_options(to_type = timestamp(unit = "s")))
+ time2 <- build_expr("cast", time2, options = cast_options(to_type = timestamp(unit = "s")))
+ } else {
+ time1 <- build_expr("cast", time1, options = cast_options(to_type = timestamp(timezone = tz, unit = "s")))
+ time2 <- build_expr("cast", time2, options = cast_options(to_type = timestamp(timezone = tz, unit = "s")))
+ }
+ }
+
+ build_expr("cast", time1 - time2, options = cast_options(to_type = duration("s")))
+ })
+
+ register_binding("as.difftime", function(x,
+ format = "%X",
+ units = "auto",
+ tz = "UTC") {
+ # windows doesn't seem to like "%X"
+ if (format == "%X" & tolower(Sys.info()[["sysname"]]) == "windows") {
+ format <- "%H:%M:%S"
+ }
+
+ if (units != "secs") {
+ abort("`as.difftime()` with units other than seconds not supported in Arrow")
+ }
+
+ if (call_binding("is.character", x)) {
+ x <- build_expr("strptime", x, options = list(format = format, tz = tz, unit = 0L))
+ y <- build_expr("strptime", "0:0:0", options = list(format = "%H:%M:%S", tz = tz, unit = 0L))
+ diff_x_y <- call_binding("difftime", x, y, units = "secs", tz = tz)
+ return(diff_x_y)
+ }
Review comment:
We should dig into + test this conversion when DST happens — I _think_ that's the only time that tz would be relevant when someone is trying to convert `04:00:00` into a difftime.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org