jonkeane commented on a change in pull request #12319:
URL: https://github.com/apache/arrow/pull/12319#discussion_r822795609
##########
File path: r/R/dplyr-funcs-type.R
##########
@@ -292,3 +293,18 @@ register_bindings_type_elementwise <- function() {
is_inf & !call_binding("is.na", is_inf)
})
}
+
+register_bindings_type_format <- function() {
+ register_binding("format", function(x, ...) {
+ if (!inherits(x, "Expression")) {
+ return(format(x, ...))
+ }
Review comment:
```suggestion
# We use R's format if we get a single R object here since we don't
(yet) support all
# of the possible options for casting to string.
if (!inherits(x, "Expression")) {
return(format(x, ...))
}
```
##########
File path: r/tests/testthat/test-dplyr-funcs-type.R
##########
@@ -843,3 +843,105 @@ test_that("as.Date() converts successfully from date,
timestamp, integer, char a
test_df
)
})
+
+test_that("format date/time", {
+ skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168
+
+ times <- tibble(
+ datetime = c(lubridate::ymd_hms("2018-10-07 19:04:05", tz =
"Pacific/Marquesas"), NA),
+ date = c(as.Date("2021-01-01"), NA)
+ )
+ formats <- "%a %A %w %d %b %B %m %y %Y %H %I %p %M %z %Z %j %U %W %x %X %%
%G %V %u"
+ formats_date <- "%a %A %w %d %b %B %m %y %Y %H %I %p %M %j %U %W %x %X %% %G
%V %u"
+
+ compare_dplyr_binding(
+ .input %>%
+ mutate(x = format(datetime, format = formats)) %>%
+ collect(),
+ times
+ )
+
+ compare_dplyr_binding(
+ .input %>%
+ mutate(x = format(date, format = formats_date)) %>%
+ collect(),
+ times
+ )
+
+ compare_dplyr_binding(
+ .input %>%
+ mutate(x = format(datetime, format = formats, tz = "Europe/Bucharest"))
%>%
+ collect(),
+ times
+ )
+
+ compare_dplyr_binding(
+ .input %>%
+ mutate(x = format(datetime, format = formats, tz = "EST", usetz = TRUE))
%>%
+ collect(),
+ times
+ )
+
+ compare_dplyr_binding(
+ .input %>%
+ mutate(x = format(1),
+ y = format(13.7, nsmall = 3)) %>%
+ collect(),
+ times
+ )
+
+ compare_dplyr_binding(
+ .input %>%
+ mutate(start_date = format(as.POSIXct("2022-01-01 01:01:00"))) %>%
+ collect(),
+ times
+ )
+
+ withr::with_timezone(
+ "Pacific/Marquesas",
+ {
+ compare_dplyr_binding(
+ .input %>%
+ mutate(
+ x = format(datetime, format = formats, tz = "EST"),
+ x_date = format(date, format = formats_date, tz = "EST")
+ ) %>%
+ collect(),
+ times
+ )
+
+ compare_dplyr_binding(
+ .input %>%
+ mutate(
+ x = format(datetime, format = formats),
+ x_date = format(date, format = formats_date)
+ ) %>%
+ collect(),
+ times
+ )
+ }
+ )
+})
+
+test_that("format() for unsupported types returns the input as string", {
+ expect_equal(
+ example_data %>%
+ record_batch() %>%
+ mutate(x = format(int, trim = TRUE)) %>%
Review comment:
Does this `trim = TRUE` actually get evaluated here, or is it being
ignored? I think from the code above it's being ignored (since this is being
evaluated in arrow I believe) but I could be wrong...
If it is being ignored, we probably should take it out here so it doesn't
confuse someone in the future that it's important or being used.
(and same for `nsmall` down below)
##########
File path: r/R/dplyr-funcs-type.R
##########
@@ -292,3 +293,18 @@ register_bindings_type_elementwise <- function() {
is_inf & !call_binding("is.na", is_inf)
})
}
+
+register_bindings_type_format <- function() {
+ register_binding("format", function(x, ...) {
+ if (!inherits(x, "Expression")) {
+ return(format(x, ...))
+ }
Review comment:
How does this sound? Since this is a bit unusual that we _aren't_
quickly converting as scalar to Arrow and then executing, having a comment as
to why is good to have.
--
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]
