jonkeane commented on a change in pull request #10519:
URL: https://github.com/apache/arrow/pull/10519#discussion_r652126604
##########
File path: r/tests/testthat/test-dplyr-arrange.R
##########
@@ -139,18 +139,14 @@ test_that("arrange() on integer, double, and character
columns", {
collect(),
tbl
)
- expect_warning(
- expect_equal(
- tbl %>%
- Table$create() %>%
- arrange(abs(int), dbl) %>%
- collect(),
- tbl %>%
- arrange(abs(int), dbl) %>%
- collect()
- ),
- "not supported in Arrow",
- fixed = TRUE
+ expect_equal(
Review comment:
If we're going to keep this, could we use `expect_dplyr_equal()` here as
well?
Though I'm not certain we really need to have this test since we test that
`abs()` works in test-dplyr.R. It might still be good/nice to have a test that
confirms the warning when a function is not supported in Arrow which in some
ways looks like what this test was really intending to test. Maybe instead of
using a function like this that we might make a kernel for we make up some
other function that we will never use in a kernel (at least the name) and keep
the test that throws a warning + pulls things into R + compares that the
results are equal.
##########
File path: r/R/dplyr-functions.R
##########
@@ -108,6 +108,10 @@ nse_funcs$is.infinite <- function(x) {
is_inf & !nse_funcs$is.na(is_inf)
}
+nse_funcs$abs <- function(x){
Review comment:
A super nit-picky whitespace suggestion:
```suggestion
nse_funcs$abs <- function(x) {
```
--
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]
