[GitHub] [arrow] jonkeane commented on a change in pull request #12357: ARROW-14817 [R] Implement bindings for lubridate::tz

[GitBox <gi...@apache.org>]

jonkeane commented on a change in pull request #12357:
URL: https://github.com/apache/arrow/pull/12357#discussion_r801986202



##########
File path: r/R/dplyr-funcs-datetime.R
##########
@@ -147,5 +147,12 @@ register_bindings_datetime <- function() {
   register_binding("pm", function(x) {
     !call_binding("am", x)
   })
-
+  register_binding("tz", function(x) {
+    if (call_binding("is.character", x) ||
+        call_binding("is.Date", x)) {
+      return("UTC")

Review comment:
       Hmm, I know that this is supported in lubridate, though it looks like even there it's only for backwards compatibility: 
   
   https://github.com/tidyverse/lubridate/blob/566590f51364e6c42251cc1721f37c314ddf7e5f/R/accessors-tz.r#L21-L22
   
   I wonder if we should add this and keep on maintaining compatibility with lubridate on a feature that they only have to maintain backwards compatibility is the right way to go... It is especially strange that something like `tz("foo")` or `tz(NA)` would return `UTC`, but even dates don't really line up to UTC (or rather: you can imagine conceptualizing a date with a timezone attached, so asserting that they are all UTC isn't quite right...)

##########
File path: r/R/dplyr-funcs-datetime.R
##########
@@ -147,5 +147,12 @@ register_bindings_datetime <- function() {
   register_binding("pm", function(x) {
     !call_binding("am", x)
   })
-
+  register_binding("tz", function(x) {
+    if (call_binding("is.character", x) ||
+        call_binding("is.Date", x)) {
+      return("UTC")
+    } else {
+      return(x$type()$timezone())
+    }

Review comment:
       What happens if you call this binding on an integer or a float? 




-- 
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