nealrichardson commented on code in PR #41576:
URL: https://github.com/apache/arrow/pull/41576#discussion_r1616405343
##########
r/R/dplyr-arrange.R:
##########
@@ -19,47 +19,46 @@
# The following S3 methods are registered on load if dplyr is present
arrange.arrow_dplyr_query <- function(.data, ..., .by_group = FALSE) {
- call <- match.call()
- .data <- as_adq(.data)
- exprs <- expand_across(.data, quos(...))
+ try_arrow_dplyr({
+ .data <- as_adq(.data)
+ exprs <- expand_across(.data, quos(...))
- if (.by_group) {
- # when the data is grouped and .by_group is TRUE, order the result by
- # the grouping columns first
- exprs <- c(quos(!!!dplyr::groups(.data)), exprs)
- }
- if (length(exprs) == 0) {
- # Nothing to do
- return(.data)
- }
- .data <- as_adq(.data)
- # find and remove any dplyr::desc() and tidy-eval
- # the arrange expressions inside an Arrow data_mask
- sorts <- vector("list", length(exprs))
- descs <- logical(0)
- mask <- arrow_mask(.data)
- for (i in seq_along(exprs)) {
- x <- find_and_remove_desc(exprs[[i]])
- exprs[[i]] <- x[["quos"]]
- sorts[[i]] <- arrow_eval(exprs[[i]], mask)
- names(sorts)[i] <- format_expr(exprs[[i]])
- if (inherits(sorts[[i]], "try-error")) {
- msg <- paste("Expression", names(sorts)[i], "not supported in Arrow")
- return(abandon_ship(call, .data, msg))
Review Comment:
Here's an example of "not just an indentation change": in the new code, we
don't have to evaluate, catch the error, and re-raise in abandon_ship, we just
let `arrow_eval()` raise, and `try_arrow_dplyr()` handles the abandon_ship.
--
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]
