ARROW-9187: [R] Add bindings for arithmetic kernels #8947
Conversation
…rrow into 10322-Minimize-Expression-to-a-
|
|
||
| expect_dplyr_equal( | ||
| input %>% | ||
| filter(dbl %/% 2 > 3) %>% |
There was a problem hiding this comment.
All these test are with dbl; should we add some with int or other columns? Or do you think the other types are better tested elsewhere (test-compute-arith.R) and this is really just testing the dplyr NSE?
r/R/expression.R
Outdated
| return(out) | ||
| } | ||
|
|
||
| # hack to use subtract instead of subtract_checked for timestamps |
There was a problem hiding this comment.
Why only subtract?
And technically this could also be else if from above
| b <- Scalar$create(Sys.time()) | ||
| result <- a - b | ||
| expect_is(result$type, "DataType") | ||
| expect_identical(result$type$ToString(), "duration[us]") |
There was a problem hiding this comment.
(1) I don't think this is right. timestamp + integer = timestamp, but because integer is cast to timestamp, you get duration.
(1b) Maybe you could support that by casting the integer to duration (same units as the timestamp), but this is a bunch of stuff that should probably get handled in C++
(2) According to https://arrow.apache.org/docs/r/articles/arrow.html#arrow-to-r, we don't support converting duration types to R.
Given this, I think arithmetic with dates/times/timestamps should be punted to its own JIRA.
There was a problem hiding this comment.
Ok, I've made ARROW-11090 — I'm honestly not sure these tests are even worth keeping + skipping with that since we will almost certainly make better support for durations (and other time-related types that don't have as much support) arrow > R at that point.
|
Rebase wasn't clean so I opened #9117 with the commits cherry-picked. |
Based on ARROW-10322