jonkeane commented on code in PR #12855:
URL: https://github.com/apache/arrow/pull/12855#discussion_r855484022
##########
r/tests/testthat/test-dplyr-funcs-datetime.R:
##########
@@ -1303,6 +1303,74 @@ test_that("dminutes, dhours, ddays, dweeks, dmonths,
dyears", {
tibble(),
ignore_attr = TRUE
)
+
+ # double -> duration not supported in Arrow.
+ # Error is generated in the C++ code
+ expect_error(
+ test_df %>%
+ arrow_table() %>%
+ mutate(r_obj_dminutes = dminutes(1.12345)) %>%
+ collect()
+ )
Review Comment:
Thanks for this comment about why we are `expect_error()` but not actually
asserting it (since this is all C++). 💯
##########
r/R/dplyr-funcs-datetime.R:
##########
@@ -353,6 +353,44 @@ register_bindings_duration <- function() {
})
}
+.helpers_function_map <- list(
+ "dminutes" = list(60, "s"),
+ "dhours" = list(3600, "s"),
+ "ddays" = list(86400, "s"),
+ "dweeks" = list(604800, "s"),
+ "dmonths" = list(2629800, "s"),
+ "dyears" = list(31557600, "s"),
+ "dseconds" = list(1, "s"),
+ "dmilliseconds" = list(1, "ms"),
+ "dmicroseconds" = list(1, "us"),
+ "dnanoseconds" = list(1, "ns")
+)
+make_duration <- function(x, unit) {
+ x <- build_expr("cast", x, options = cast_options(to_type = int64()))
+ x$cast(duration(unit))
+}
+register_bindings_duration_helpers <- function() {
+ duration_helpers_map_factory <- function(value, unit) {
+ force(value)
+ force(unit)
+ function(x = 1) make_duration(x * value, unit)
+ }
+
+ for (name in names(.helpers_function_map)) {
+ register_binding(
+ name,
+ duration_helpers_map_factory(
+ .helpers_function_map[[name]][[1]],
+ .helpers_function_map[[name]][[2]]
+ )
+ )
+ }
Review Comment:
Nice! This is actually even shorter than I though it would be!
##########
r/tests/testthat/test-dplyr-funcs-datetime.R:
##########
@@ -1303,6 +1303,74 @@ test_that("dminutes, dhours, ddays, dweeks, dmonths,
dyears", {
tibble(),
ignore_attr = TRUE
)
+
+ # double -> duration not supported in Arrow.
+ # Error is generated in the C++ code
+ expect_error(
+ test_df %>%
+ arrow_table() %>%
+ mutate(r_obj_dminutes = dminutes(1.12345)) %>%
+ collect()
+ )
+})
+
+test_that("dseconds, dmilliseconds, dmicroseconds, dnanoseconds,
dpicoseconds", {
+ example_d <- tibble(x = c(1:10, NA))
+ date_to_add <- ymd("2009-08-03", tz = "Pacific/Marquesas")
+
+ compare_dplyr_binding(
+ .input %>%
+ mutate(
+ dseconds = dseconds(x),
+ dmilliseconds = dmilliseconds(x),
+ dmicroseconds = dmicroseconds(x),
+ dnanoseconds = dnanoseconds(x),
+ ) %>%
+ collect(),
+ example_d,
+ ignore_attr = TRUE
+ )
+
+ compare_dplyr_binding(
+ .input %>%
+ mutate(
+ dseconds = dseconds(x),
+ dmicroseconds = dmicroseconds(x),
+ new_date_1 = date_to_add + dseconds,
+ new_date_2 = date_to_add + dseconds - dmicroseconds,
+ new_duration = dseconds - dmicroseconds
+ ) %>%
+ collect(),
+ example_d,
+ ignore_attr = TRUE
Review Comment:
I didn't see this in the PR (though might have missed something), what
`attr` are we ignoring? Maybe we should add a comment about what we're using
that for
--
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]
