Skip to content

ARROW-13501: [R] Bindings for count aggregation#10989

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nealrichardson wants to merge 3 commits into from
Closed

ARROW-13501: [R] Bindings for count aggregation #10989

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3d7badc
Start implementing dplyr::n() (blocked by segfault)
nealrichardson Aug 24, 2021
d436428
Unskip test and lint
nealrichardson Aug 30, 2021
2ccd6e3
Move the case_when skip forward
nealrichardson Aug 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions r/R/dplyr-functions.R
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -825,3 +825,10 @@ agg_funcs$var <- function(x, na.rm = FALSE, ddof = 1) {
options = list(ddof = ddof)
)
}
agg_funcs$n <- function() {
list(
fun = "sum",
data = Expression$scalar(1L),
options = list()
)
}
8 changes: 6 additions & 2 deletions r/src/compute.cpp
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -177,8 +177,12 @@ std::shared_ptr<arrow::compute::FunctionOptions> make_compute_options(
func_name == "hash_all") {
using Options = arrow::compute::ScalarAggregateOptions;
auto out = std::make_shared<Options>(Options::Defaults());
out->min_count = cpp11::as_cpp<int>(options["na.min_count"]);
out->skip_nulls = cpp11::as_cpp<bool>(options["na.rm"]);
if (!Rf_isNull(options["na.min_count"])) {
out->min_count = cpp11::as_cpp<int>(options["na.min_count"]);
}
if (!Rf_isNull(options["na.rm"])) {
out->skip_nulls = cpp11::as_cpp<bool>(options["na.rm"]);
}
return out;
}

Expand Down
20 changes: 18 additions & 2 deletions r/tests/testthat/test-dplyr-aggregate.R
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -154,10 +154,26 @@ test_that("Group by var on dataset", {
)
})


test_that("Group by any/all", {
test_that("n()", {
withr::local_options(list(arrow.debug = TRUE))
expect_dplyr_equal(
input %>%
summarize(counts = n()) %>%
collect(),
tbl
)

expect_dplyr_equal(
input %>%
group_by(some_grouping) %>%
summarize(counts = n()) %>%
arrange(some_grouping) %>%
collect(),
tbl
)
})

test_that("Group by any/all", {
expect_dplyr_equal(
input %>%
group_by(some_grouping) %>%
Expand Down
10 changes: 4 additions & 6 deletions r/tests/testthat/test-dplyr.R
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -26,6 +26,7 @@ tbl$verses <- verses[[1]]
# c(" a ", " b ", " c ", ...) increasing padding
# nchar = 3 5 7 9 11 13 15 17 19 21
tbl$padded_strings <- stringr::str_pad(letters[1:10], width = 2 * (1:10) + 1, side = "both")
tbl$another_chr <- tail(letters, 10)

test_that("basic select/filter/collect", {
batch <- record_batch(tbl)
Expand DownExpand Up@@ -961,7 +962,7 @@ test_that("No duplicate field names are allowed in an arrow_dplyr_query", {
filter(int > 0),
regexp = paste0(
'The following field names were found more than once in the data: "int", "dbl", ',
'"dbl2", "lgl", "false", "chr", "fct", "verses", and "padded_strings"'
'"dbl2", "lgl", "false", "chr", "fct", "verses", "padded_strings"'
)
)
})
Expand DownExpand Up@@ -1109,9 +1110,6 @@ test_that("trig functions", {
})

test_that("if_else and ifelse", {
tbl <- example_data
tbl$another_chr <- tail(letters, 10)

expect_dplyr_equal(
input %>%
mutate(
Expand DownExpand Up@@ -1342,7 +1340,6 @@ test_that("case_when()", {
)
)

skip("case_when does not yet support with variable-width types (ARROW-13222)")
expect_dplyr_equal(
input %>%
transmute(cw = case_when(lgl ~ "abc")) %>%
Expand All@@ -1355,10 +1352,11 @@ test_that("case_when()", {
collect(),
tbl
)
skip("ARROW-13799: factor() should error but instead we get a string error message in its place")
expect_dplyr_equal(
input %>%
mutate(
cw = paste0(case_when(!(!(!(lgl))) ~ factor(chr), TRUE ~ fct), "!")
cw = case_when(!(!(!(lgl))) ~ factor(chr), TRUE ~ fct)
) %>%
collect(),
tbl
Expand Down