kszucs commented on code in PR #45001:
URL: https://github.com/apache/arrow/pull/45001#discussion_r3641444562
##########
r/tests/testthat/test-compute-aggregate.R:
##########
@@ -21,7 +21,7 @@ test_that("list_compute_functions", {
justmins <- list_compute_functions("^min")
expect_true(length(justmins) > 0)
expect_all_true(grepl("min", justmins))
- no_hash_funcs <- list_compute_functions("^hash")
+ no_hash_funcs <- list_compute_functions("^hash_")
expect_true(length(no_hash_funcs) == 0)
Review Comment:
This test's own `^hash_` filter is by design unrelated to `hash32`/`hash64`
— it verifies that `list_compute_functions()` hides the group-by aggregations
(`hash_count`, `hash_sum`, etc., see `list_compute_functions()` in
r/R/compute.R, which unconditionally strips `^hash_` names). Since
`hash32`/`hash64` have no underscore, they were never filtered by that
mechanism and remain fully discoverable — so this specific line isn't broken.
That said, you're right there was no R-level assertion of that discoverability,
so I added one alongside it: `expect_true(all(c("hash32", "hash64") %in%
allfuncs))`.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]