nealrichardson commented on a change in pull request #9674:
URL: https://github.com/apache/arrow/pull/9674#discussion_r591969663
##########
File path: r/tests/testthat/test-dplyr-filter.R
##########
@@ -155,6 +155,15 @@ test_that("filter() with %in%", {
)
})
+test_that("filter() with between()", {
+ expect_dplyr_equal(
+ input %>%
+ filter(between(dbl, 1, 2)) %>%
+ collect(),
+ tbl
+ )
+})
+
Review comment:
Should also add a test for the input validation.
When you do, one question: what happens if you remove the input validation
and let the Arrow C++ library handle it? Are the error messages clear enough?
Removing the R validation would allow `left` and `right` to be other kinds
of objects (such as Arrow objects). It would also support vector-wise
comparison (i.e. if `left` and `right` were columns in the data), which might
be nice.
##########
File path: r/R/dplyr.R
##########
@@ -299,6 +299,15 @@ build_function_list <- function(FUN) {
both = FUN("utf8_trim_whitespace", string)
)
},
+ between = function(x, left, right) {
+ if (length(left) != 1) {
+ rlang::abort("`left` must be length 1")
Review comment:
We import `abort` from `rlang` so you can drop `rlang::`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
