jonkeane commented on a change in pull request #11777:
URL: https://github.com/apache/arrow/pull/11777#discussion_r758614090
##########
File path: r/tests/testthat/test-dplyr-summarize.R
##########
@@ -738,7 +742,7 @@ test_that("Not (yet) supported: implicit join", {
) %>%
collect(),
tbl,
- warning = "Expression sum\\(\\(dbl - mean\\(dbl\\)\\)\\^2\\) not supported
in Arrow; pulling data into R"
+ warning = "Aggregate within aggregate expression sum\\(\\(dbl -
mean\\(dbl\\)\\)\\^2\\) not supported in Arrow; pulling data into R"
Review comment:
Could we split this onto two lines to appease the linter?
##########
File path: r/tests/testthat/test-dplyr-summarize.R
##########
@@ -748,7 +752,7 @@ test_that("Not (yet) supported: implicit join", {
) %>%
collect(),
tbl,
- warning = "Expression sum\\(dbl - mean\\(dbl\\)\\) not supported in Arrow;
pulling data into R"
+ warning = "Aggregate within aggregate expression sum\\(dbl -
mean\\(dbl\\)\\) not supported in Arrow; pulling data into R"
Review comment:
Same thing here on line length
##########
File path: r/R/dplyr-summarize.R
##########
@@ -166,14 +166,37 @@ format_aggregation <- function(x) {
# appropriate combination of (1) aggregations (possibly temporary) and
# (2) post-aggregation transformations (mutate)
# The function returns nothing: it assigns into the `ctx` environment
-summarize_eval <- function(name, quosure, ctx, hash, recurse = FALSE) {
+summarize_eval <- function(name, quosure, ctx, hash) {
Review comment:
`recurse` was not used anywhere in this function, so you cleaned it up
from here, yeah?
##########
File path: r/tests/testthat/test-dplyr-summarize.R
##########
@@ -758,7 +762,7 @@ test_that("Not (yet) supported: implicit join", {
) %>%
collect(),
tbl,
- warning = "Expression sum\\(\\(dbl - mean\\(dbl\\)\\)\\^2\\) not supported
in Arrow; pulling data into R"
+ warning = "Aggregate within aggregate expression sum\\(\\(dbl -
mean\\(dbl\\)\\)\\^2\\) not supported in Arrow; pulling data into R"
Review comment:
Same here
##########
File path: r/tests/testthat/test-dplyr-summarize.R
##########
@@ -769,7 +773,18 @@ test_that("Not (yet) supported: implicit join", {
) %>%
collect(),
tbl,
- warning = "Expression dbl - mean\\(dbl\\) not supported in Arrow; pulling
data into R"
+ warning = "Expression dbl - mean\\(dbl\\) is not an aggregate expression
or is not supported in Arrow; pulling data into R"
Review comment:
One last one
##########
File path: r/tests/testthat/test-dplyr-summarize.R
##########
@@ -896,3 +911,47 @@ test_that("summarise() passes through type information for
temporary columns", {
)
)
})
+
+test_that("summarise() can handle scalars and literal values", {
+ some_scalar_value = 2L
Review comment:
```suggestion
some_scalar_value <- 2L
```
--
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]
