From 9c7d25e2e01ffd92c413c4332207f851c0f0aeee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 30 Mar 2022 16:39:56 +0100 Subject: [PATCH 01/19] first pass at `make_difftime` binding --- r/R/dplyr-funcs-datetime.R | 49 ++++++++++++++++++++ r/tests/testthat/test-dplyr-funcs-datetime.R | 45 ++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/r/R/dplyr-funcs-datetime.R b/r/R/dplyr-funcs-datetime.R index c583aed5472..1a0e43b17cc 100644 --- a/r/R/dplyr-funcs-datetime.R +++ b/r/R/dplyr-funcs-datetime.R @@ -309,6 +309,25 @@ register_bindings_duration <- function() { build_expr("cast", x, options = cast_options(to_type = duration(unit = "s"))) }) + register_binding("make_difftime", function(num = NULL, + units = "secs", + ...) { + if (units != "secs") { + abort("`make_difftime()` with units other than 'secs' not supported in Arrow") + } + + chunks <- list(...) + + if (is.null(num)) { + duration <- duration_from_chunks(chunks) + } else if (length(chunks) == 0){ + duration <- num + } else { + duration <- num + duration_from_chunks(chunks) + } + duration <- build_expr("cast", duration, options = cast_options(to_type = int64())) + duration$cast(duration("s")) + }) } binding_format_datetime <- function(x, format = "", tz = "", usetz = FALSE) { @@ -330,3 +349,33 @@ binding_format_datetime <- function(x, format = "", tz = "", usetz = FALSE) { build_expr("strftime", x, options = list(format = format, locale = Sys.getlocale("LC_TIME"))) } + +duration_from_chunks <- function(chunks) { + accepted_chunks <- c("second", "minute", "hour", "day", "week") + matched_chunks <- accepted_chunks[pmatch(names(chunks), accepted_chunks, duplicates.ok = TRUE)] + + if (any(is.na(matched_chunks))) { + abort( + paste0( + "Invalid `difftime` parts: ", + oxford_paste(names(chunks[is.na(matched_chunks)]), quote_symbol = "`") + ) + ) + } + + matched_chunks <- matched_chunks[!is.na(matched_chunks)] + + chunks <- chunks[matched_chunks] + chunk_duration <- c( + "second" = 1L, + "minute" = 60L, + "hour" = 3600L, + "day" = 86400L, + "week" = 604800L + ) + duration <- 0L + for (chunk in names(chunks)) { + duration <- duration + chunks[[chunk]] * chunk_duration[[chunk]] + } + duration + } diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 6df40505d1a..4524f1460ff 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -1186,3 +1186,48 @@ test_that("as.difftime()", { collect() ) }) + +test_that("make_difftime()", { + test_df <- tibble( + seconds = c(3, 4, 5, 6), + minutes = c(1.5, 2.3, 4.5, 6.7), + hours = c(2, 3, 4, 5), + days = c(6, 7, 8, 9), + weeks = c(1, 3, 5, NA), + number = 10:13 + ) + + compare_dplyr_binding( + .input %>% + mutate( + duration_from_parts = make_difftime( + second = seconds, + minute = minutes, + hour = hours, + day = days, + week = weeks, + units = "secs"), + duration_from_num = make_difftime( + num = number, + units = "secs" + ), + duration_from_r_obj = make_difftime( + num = 154, + units = "secs" + ), + duration_from_r_chunks = make_difftime( + minute = 45, + day = 2, + week = 4, + units = "secs" + ) + ) %>% + collect(), + test_df + ) + + test_df %>% + arrow_table() %>% + mutate(error_difftime = make_difftime(month = number, units = "secs")) %>% + collect() +}) From 4533c0926d3729cb25ca2f6ff0f04c30e1d20d6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Thu, 31 Mar 2022 09:51:46 +0100 Subject: [PATCH 02/19] lint + error expectations --- r/R/dplyr-funcs-datetime.R | 2 +- r/tests/testthat/test-dplyr-funcs-datetime.R | 24 +++++++++++++++----- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/r/R/dplyr-funcs-datetime.R b/r/R/dplyr-funcs-datetime.R index 1a0e43b17cc..4fd8576b61b 100644 --- a/r/R/dplyr-funcs-datetime.R +++ b/r/R/dplyr-funcs-datetime.R @@ -320,7 +320,7 @@ register_bindings_duration <- function() { if (is.null(num)) { duration <- duration_from_chunks(chunks) - } else if (length(chunks) == 0){ + } else if (length(chunks) == 0) { duration <- num } else { duration <- num + duration_from_chunks(chunks) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 4524f1460ff..6bb285fbc5a 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -1211,11 +1211,11 @@ test_that("make_difftime()", { num = number, units = "secs" ), - duration_from_r_obj = make_difftime( + duration_from_r_num = make_difftime( num = 154, units = "secs" ), - duration_from_r_chunks = make_difftime( + duration_from_r_parts = make_difftime( minute = 45, day = 2, week = 4, @@ -1226,8 +1226,20 @@ test_that("make_difftime()", { test_df ) - test_df %>% - arrow_table() %>% - mutate(error_difftime = make_difftime(month = number, units = "secs")) %>% - collect() + # month as component not supported due to variable length + expect_error( + test_df %>% + arrow_table() %>% + mutate(error_difftime = make_difftime(month = number, units = "secs")) %>% + collect() + ) + + # units other than "secs" not supported since they are the only ones in common + # between R and Arrow + expect_error( + test_df %>% + arrow_table() %>% + mutate(error_difftime = make_difftime(num = number, units = "mins")) %>% + collect() + ) }) From 889d75cc1cdc6e1635bed38a660cd27bf3fa1a6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Fri, 1 Apr 2022 12:30:34 +0100 Subject: [PATCH 03/19] simpler testing --- r/tests/testthat/test-dplyr-funcs-datetime.R | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 6bb285fbc5a..24578962583 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -1226,20 +1226,13 @@ test_that("make_difftime()", { test_df ) - # month as component not supported due to variable length - expect_error( - test_df %>% - arrow_table() %>% - mutate(error_difftime = make_difftime(month = number, units = "secs")) %>% - collect() - ) - # units other than "secs" not supported since they are the only ones in common # between R and Arrow - expect_error( - test_df %>% - arrow_table() %>% + compare_dplyr_binding( + .input %>% mutate(error_difftime = make_difftime(num = number, units = "mins")) %>% - collect() + collect(), + test_df, + warning = TRUE ) }) From a43bf018d5f060140c996e5ce2a5018915e61ac3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Tue, 12 Apr 2022 16:56:23 +0300 Subject: [PATCH 04/19] used `purrr::imap()` + `purrr::reduce()` instead of the `for` loop. --- r/R/dplyr-funcs-datetime.R | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/r/R/dplyr-funcs-datetime.R b/r/R/dplyr-funcs-datetime.R index 4fd8576b61b..fe5eb361807 100644 --- a/r/R/dplyr-funcs-datetime.R +++ b/r/R/dplyr-funcs-datetime.R @@ -373,9 +373,7 @@ duration_from_chunks <- function(chunks) { "day" = 86400L, "week" = 604800L ) - duration <- 0L - for (chunk in names(chunks)) { - duration <- duration + chunks[[chunk]] * chunk_duration[[chunk]] - } - duration + chunks_total <- purrr::imap(chunks, ~.x * chunk_duration[[.y]]) %>% + purrr::reduce(`+`) + chunks_total } From ac27103e7f283683404bb0880eae206c7bed886c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Tue, 12 Apr 2022 17:32:48 +0300 Subject: [PATCH 05/19] added comment on the purpose of `duration_from_chunks()` --- r/R/dplyr-funcs-datetime.R | 2 ++ 1 file changed, 2 insertions(+) diff --git a/r/R/dplyr-funcs-datetime.R b/r/R/dplyr-funcs-datetime.R index fe5eb361807..a699f433f45 100644 --- a/r/R/dplyr-funcs-datetime.R +++ b/r/R/dplyr-funcs-datetime.R @@ -350,6 +350,8 @@ binding_format_datetime <- function(x, format = "", tz = "", usetz = FALSE) { build_expr("strftime", x, options = list(format = format, locale = Sys.getlocale("LC_TIME"))) } +# this is a helper function used for creating a difftime / duration objects from +# several of the accepted pieces (second, minute, hour, day, week) duration_from_chunks <- function(chunks) { accepted_chunks <- c("second", "minute", "hour", "day", "week") matched_chunks <- accepted_chunks[pmatch(names(chunks), accepted_chunks, duplicates.ok = TRUE)] From b3f2dc1844a35ce223f801dbe0708b3d0bba15b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 13 Apr 2022 11:12:00 +0300 Subject: [PATCH 06/19] abort when both quantities are passed both via `num` and `...` + test --- r/R/dplyr-funcs-datetime.R | 2 +- r/tests/testthat/test-dplyr-funcs-datetime.R | 25 +++++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/r/R/dplyr-funcs-datetime.R b/r/R/dplyr-funcs-datetime.R index a699f433f45..3a9b0e08b13 100644 --- a/r/R/dplyr-funcs-datetime.R +++ b/r/R/dplyr-funcs-datetime.R @@ -323,7 +323,7 @@ register_bindings_duration <- function() { } else if (length(chunks) == 0) { duration <- num } else { - duration <- num + duration_from_chunks(chunks) + abort("`make_difftime()` with both `num` and `...` not supported in Arrow") } duration <- build_expr("cast", duration, options = cast_options(to_type = int64())) duration$cast(duration("s")) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 24578962583..97644e51af7 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -1206,7 +1206,8 @@ test_that("make_difftime()", { hour = hours, day = days, week = weeks, - units = "secs"), + units = "secs" + ), duration_from_num = make_difftime( num = number, units = "secs" @@ -1235,4 +1236,26 @@ test_that("make_difftime()", { test_df, warning = TRUE ) + + # constructing a difftime from both `num` and parts passed through `...` while + # possible with the lubridate function (resulting in a concatenation of the 2 + # resulting objects), it errors in a dplyr context + expect_error( + expect_warning( + test_df %>% + arrow_table() %>% + mutate( + duration_from_num_and_parts = make_difftime( + num = number, + second = seconds, + minute = minutes, + hour = hours, + day = days, + week = weeks, + units = "secs" + ) + ) %>% + collect() + ) + ) }) From e10daf2a9d3581ed081f39907fdff01f0d81e9b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 13 Apr 2022 11:19:16 +0300 Subject: [PATCH 07/19] post merge conflict clean-up --- r/tests/testthat/test-dplyr-funcs-datetime.R | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index bfee6ba0f7c..1912b7ba2f4 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -1262,8 +1262,8 @@ test_that("make_difftime()", { days = c(6, 7, 8, 9), weeks = c(1, 3, 5, NA), number = 10:13 - ) - + ) + compare_dplyr_binding( .input %>% mutate( @@ -1323,4 +1323,6 @@ test_that("make_difftime()", { ) ) %>% collect() - ) \ No newline at end of file + ) + ) +}) From 08a06f9705f8dec9ebb71482f9b355371e344009 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 13 Apr 2022 12:02:09 +0300 Subject: [PATCH 08/19] improved the error message for unsupported difftime parts + test --- r/R/dplyr-funcs-datetime.R | 4 +++- r/tests/testthat/test-dplyr-funcs-datetime.R | 13 +++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/r/R/dplyr-funcs-datetime.R b/r/R/dplyr-funcs-datetime.R index 86e235bd2f0..6d7c675a2bb 100644 --- a/r/R/dplyr-funcs-datetime.R +++ b/r/R/dplyr-funcs-datetime.R @@ -401,7 +401,9 @@ duration_from_chunks <- function(chunks) { if (any(is.na(matched_chunks))) { abort( paste0( - "Invalid `difftime` parts: ", + "named `difftime` units other than: ", + oxford_paste(accepted_chunks, quote_symbol = "`"), + " not supported in Arrow. \nInvalid `difftime` parts: ", oxford_paste(names(chunks[is.na(matched_chunks)]), quote_symbol = "`") ) ) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 1912b7ba2f4..1cb58bb2f45 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -1294,6 +1294,19 @@ test_that("make_difftime()", { test_df ) + # named difftime parts other than `second`, `minute`, `hour`, `day` and `week` + # are not supported + expect_error( + expect_warning( + test_df %>% + arrow_table() %>% + mutate( + err_difftime = make_difftime(month = 2) + ) %>% + collect() + ) + ) + # units other than "secs" not supported since they are the only ones in common # between R and Arrow compare_dplyr_binding( From c78f6eeaf1acee9efa91d4f7bc90ed7581f2e74f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 13 Apr 2022 12:46:35 +0300 Subject: [PATCH 09/19] updated NEWS --- r/NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/r/NEWS.md b/r/NEWS.md index 1a1f198e0f3..671af261480 100644 --- a/r/NEWS.md +++ b/r/NEWS.md @@ -24,6 +24,7 @@ * component extraction functions: `tz()` (timezone), `semester()` (semester), `dst()` (daylight savings time indicator), `date()` (extract date), `epiyear()` (epiyear), improvements to `month()`, which now works with integer inputs. * Added `make_date()` & `make_datetime()` + `ISOdatetime()` & `ISOdate()` to create date-times from numeric representations. * Added `decimal_date()` and `date_decimal()` + * Added `make_difftime()` * date-time functionality: * Added `difftime` and `as.difftime()` * Added `as.Date()` to convert to date From ffca8c285d4058e4a351e74854ff3fa554fd4711 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 13 Apr 2022 17:32:41 +0300 Subject: [PATCH 10/19] short comment on what the purrr::imap + reduce bit does --- r/R/dplyr-funcs-datetime.R | 1 + 1 file changed, 1 insertion(+) diff --git a/r/R/dplyr-funcs-datetime.R b/r/R/dplyr-funcs-datetime.R index 6d7c675a2bb..c081f7c9253 100644 --- a/r/R/dplyr-funcs-datetime.R +++ b/r/R/dplyr-funcs-datetime.R @@ -419,6 +419,7 @@ duration_from_chunks <- function(chunks) { "day" = 86400L, "week" = 604800L ) + # transform the duration of each chunk in seconds and add everything together chunks_total <- purrr::imap(chunks, ~.x * chunk_duration[[.y]]) %>% purrr::reduce(`+`) chunks_total From 4e8e33767b37e5a271d651060df75ea57768a160 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Thu, 14 Apr 2022 09:55:32 +0300 Subject: [PATCH 11/19] updated NEWS --- r/NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/r/NEWS.md b/r/NEWS.md index 671af261480..eede8021698 100644 --- a/r/NEWS.md +++ b/r/NEWS.md @@ -24,7 +24,7 @@ * component extraction functions: `tz()` (timezone), `semester()` (semester), `dst()` (daylight savings time indicator), `date()` (extract date), `epiyear()` (epiyear), improvements to `month()`, which now works with integer inputs. * Added `make_date()` & `make_datetime()` + `ISOdatetime()` & `ISOdate()` to create date-times from numeric representations. * Added `decimal_date()` and `date_decimal()` - * Added `make_difftime()` + * Added `make_difftime()` (duration constructor) * date-time functionality: * Added `difftime` and `as.difftime()` * Added `as.Date()` to convert to date From 9e87f1cc98459c1fd8f08cbde55c02c235c14dc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 20 Apr 2022 09:33:30 +0100 Subject: [PATCH 12/19] avoided using the magrittr pipe --- r/R/dplyr-funcs-datetime.R | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/r/R/dplyr-funcs-datetime.R b/r/R/dplyr-funcs-datetime.R index c081f7c9253..4b5236c7bae 100644 --- a/r/R/dplyr-funcs-datetime.R +++ b/r/R/dplyr-funcs-datetime.R @@ -420,7 +420,6 @@ duration_from_chunks <- function(chunks) { "week" = 604800L ) # transform the duration of each chunk in seconds and add everything together - chunks_total <- purrr::imap(chunks, ~.x * chunk_duration[[.y]]) %>% - purrr::reduce(`+`) - chunks_total + chunks_total <- purrr::imap(chunks, ~.x * chunk_duration[[.y]]) + purrr::reduce(chunks_total, `+`) } From fc3743d470db209e98d6c16d82572205f0ecb78d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 20 Apr 2022 10:23:08 +0100 Subject: [PATCH 13/19] alignment + lints --- r/tests/testthat/test-dplyr-funcs-datetime.R | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 62b00697da7..0a9fd69e679 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -1314,11 +1314,11 @@ test_that("make_difftime()", { weeks = c(1, 3, 5, NA), number = 10:13 ) - - compare_dplyr_binding( + + compare_dplyr_binding( .input %>% mutate( - duration_from_parts = make_difftime( + duration_from_parts = make_difftime( second = seconds, minute = minutes, hour = hours, @@ -1388,5 +1388,5 @@ test_that("make_difftime()", { ) %>% collect() ) - ) -}) \ No newline at end of file + ) +}) From 4a08506cbe3a09b1669e385ffa9ebb9f70b43198 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 20 Apr 2022 10:32:55 +0100 Subject: [PATCH 14/19] re-organise the if-else logic for `num` + `...` inside the `make_difftime` binding --- r/R/dplyr-funcs-datetime.R | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/r/R/dplyr-funcs-datetime.R b/r/R/dplyr-funcs-datetime.R index 3a1a071d81c..230d291e3dd 100644 --- a/r/R/dplyr-funcs-datetime.R +++ b/r/R/dplyr-funcs-datetime.R @@ -360,13 +360,21 @@ register_bindings_duration <- function() { chunks <- list(...) - if (is.null(num)) { - duration <- duration_from_chunks(chunks) - } else if (length(chunks) == 0) { + # lubridate concatenates durations passed via the `num` argument with those + # passed via `...` resulting in a vector of length 2 - which is virtually + # unusable in a dplyr pipeline. Arrow errors in this situation + if (!is.null(num) && length(chunks) > 0) { + abort("`make_difftime()` with both `num` and `...` not supported in Arrow") + } + + if (!is.null(num)) { + # build duration from num if present duration <- num } else { - abort("`make_difftime()` with both `num` and `...` not supported in Arrow") + # build duration from chunks when nothing is passed via ... + duration <- duration_from_chunks(chunks) } + duration <- build_expr("cast", duration, options = cast_options(to_type = int64())) duration$cast(duration("s")) }) From fd8ccff6a8e6df9ef183cf9a7e88f21cd4e1d1fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 20 Apr 2022 10:40:11 +0100 Subject: [PATCH 15/19] testing R error messages --- r/tests/testthat/test-dplyr-funcs-datetime.R | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 0a9fd69e679..b1ff505004b 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -1354,7 +1354,9 @@ test_that("make_difftime()", { mutate( err_difftime = make_difftime(month = 2) ) %>% - collect() + collect(), + paste0("named `difftime` units other than: `second`, `minute`, `hour`,", + " `day`, and `week` not supported in Arrow.") ) ) @@ -1386,7 +1388,8 @@ test_that("make_difftime()", { units = "secs" ) ) %>% - collect() + collect(), + "with both `num` and `...` not supported in Arrow" ) ) }) From c51cbf1763f472ab83207dbdee29b04b0bce2e08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 20 Apr 2022 13:22:20 +0100 Subject: [PATCH 16/19] style --- r/R/dplyr-funcs-datetime.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/r/R/dplyr-funcs-datetime.R b/r/R/dplyr-funcs-datetime.R index 230d291e3dd..c8270c77de8 100644 --- a/r/R/dplyr-funcs-datetime.R +++ b/r/R/dplyr-funcs-datetime.R @@ -351,7 +351,7 @@ register_bindings_duration <- function() { delta <- delta$cast(int64()) start + delta$cast(duration("s")) }) - register_binding("make_difftime", function(num = NULL, + register_binding("make_difftime", function(num = NULL, units = "secs", ...) { if (units != "secs") { From 6d8229a0be727e337650e248b64b8285ac1f9044 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 20 Apr 2022 13:23:01 +0100 Subject: [PATCH 17/19] style --- r/R/dplyr-funcs-datetime.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/r/R/dplyr-funcs-datetime.R b/r/R/dplyr-funcs-datetime.R index c8270c77de8..3ad8a19ad64 100644 --- a/r/R/dplyr-funcs-datetime.R +++ b/r/R/dplyr-funcs-datetime.R @@ -455,4 +455,4 @@ duration_from_chunks <- function(chunks) { # transform the duration of each chunk in seconds and add everything together chunks_total <- purrr::imap(chunks, ~.x * chunk_duration[[.y]]) purrr::reduce(chunks_total, `+`) - } +} From a67237527ca97e3bf1017b136542cea298d95337 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 20 Apr 2022 13:32:06 +0100 Subject: [PATCH 18/19] switch back to using a `for` loop inside `duration_from_chunks()` --- r/R/dplyr-funcs-datetime.R | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/r/R/dplyr-funcs-datetime.R b/r/R/dplyr-funcs-datetime.R index 3ad8a19ad64..77267034358 100644 --- a/r/R/dplyr-funcs-datetime.R +++ b/r/R/dplyr-funcs-datetime.R @@ -452,7 +452,11 @@ duration_from_chunks <- function(chunks) { "day" = 86400L, "week" = 604800L ) + # transform the duration of each chunk in seconds and add everything together - chunks_total <- purrr::imap(chunks, ~.x * chunk_duration[[.y]]) - purrr::reduce(chunks_total, `+`) + duration <- 0 + for (chunk in names(chunks)) { + duration <- duration + chunks[[chunk]] * chunk_duration[[chunk]] + } + duration } From bb78a8d59fc7fee911bfd48889dcbcef10dd3125 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 20 Apr 2022 13:35:10 +0100 Subject: [PATCH 19/19] define `register_bindings_difftime_constructors()` and add `make_difftime` binding there --- r/R/dplyr-funcs-datetime.R | 3 +++ r/R/dplyr-funcs.R | 1 + 2 files changed, 4 insertions(+) diff --git a/r/R/dplyr-funcs-datetime.R b/r/R/dplyr-funcs-datetime.R index 77267034358..7657a79271b 100644 --- a/r/R/dplyr-funcs-datetime.R +++ b/r/R/dplyr-funcs-datetime.R @@ -351,6 +351,9 @@ register_bindings_duration <- function() { delta <- delta$cast(int64()) start + delta$cast(duration("s")) }) +} + +register_bindings_difftime_constructors <- function() { register_binding("make_difftime", function(num = NULL, units = "secs", ...) { diff --git a/r/R/dplyr-funcs.R b/r/R/dplyr-funcs.R index eaa6ed9dc67..c66ed04893d 100644 --- a/r/R/dplyr-funcs.R +++ b/r/R/dplyr-funcs.R @@ -106,6 +106,7 @@ create_binding_cache <- function() { register_bindings_aggregate() register_bindings_conditional() register_bindings_datetime() + register_bindings_difftime_constructors() register_bindings_duration() register_bindings_duration_helpers() register_bindings_math()