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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
