dragosmg commented on a change in pull request #12506:
URL: https://github.com/apache/arrow/pull/12506#discussion_r833216676
##########
File path: r/R/dplyr-funcs-datetime.R
##########
@@ -189,6 +189,65 @@ 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")
+ }
+
+ if (!missing(tz)) {
+ warn("`tz` is an optional argument to `difftime()` in R and will not be
used in Arrow")
+ }
+
+ time1 <- build_expr("cast", time1, options = cast_options(to_type =
timestamp()))
+ time2 <- build_expr("cast", time2, options = cast_options(to_type =
timestamp()))
+
+ build_expr("cast", time1 - time2, options = cast_options(to_type =
duration("s")))
+ })
+
+ register_binding("as.difftime", function(x,
+ format = "%X",
+ units = "auto") {
+ # 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, unit =
0L))
+ y <- build_expr("strptime", "0:0:0", options = list(format = "%H:%M:%S",
unit = 0L))
+ diff_x_y <- call_binding("difftime", x, y, units = "secs")
+ return(diff_x_y)
+ }
Review comment:
I think we have at least 3 options for this issue:
1. `x` only - use return `time32()` which currently is mapped to
`hms::difftime`. For version `v7.0.0` this mapping is documented, for `v 8.0.0`
I think the documentation doesn't mention it (as we now have `duration <->
difftime`. I personally am not a huge fan of this since we would need to rely
on un-documented behaviour and we have a slight mismatch in class.
2. `x` only - go through quite a long cycle of casting to get to duration
with the correct size. We need to cast twice to `int64()` and need to account
for the shift in measurement unit by dividing by 10^9. This seems cumbersome
and a bit difficult to read.
```r
x <- build_expr("strptime", x, options = list(format = format, unit = 0L))
x <- build_expr("/", x$cast(time64())$cast(int64()), Scalar$create(10^9,
int64()))
x <- x$cast(int64())$cast(duration("s"))
```
3. `x` and `y`. Subtract `"0:0:0"` from x (my initial approach). I still
think this is the most elegant solution.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
