jonkeane commented on a change in pull request #10237:
URL: https://github.com/apache/arrow/pull/10237#discussion_r625876806
##########
File path: r/tests/testthat/test-compute-aggregate.R
##########
@@ -37,7 +37,8 @@ test_that("sum.Array", {
floats <- c(floats, NA)
na <- Array$create(floats)
- expect_identical(as.numeric(sum(na)), sum(floats))
+ # expect_identical(as.numeric(sum(na)), sum(floats))
+ expect_identical(as.numeric(sum(na)), NA_real_)
Review comment:
Yeah, I'll add the devel check to `skip_on_linux_cran()` (and probably
rename it too, though I'm a bit hesitant to make it _so_ obvious that we are
skipping the problematic tests — is that something we should worry about?).
As for these specific tests, looking at it now I wonder if we have benefit
from `expect_identical(as.numeric(sum(na)), sum(floats))` versus
`expect_identical(as.numeric(sum(na)), NA_real_)`? Is there a circumstance
where we expect `sum(floats)` to change behavior and we would want to match it?
I find `expect_identical(as.numeric(sum(na)), NA_real_)` to be easier to read
at a glance and see exactly what it's testing/expecting.
--
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]
