Skip to content

Commit

Permalink
MINOR: [R] Work around test failure in tidyquery revdep (#43498)
Browse files Browse the repository at this point in the history
### Rationale for this change

See #43317 (comment). `tidyquery` is assembling queries in some way such that when `summarize.arrow_dplyr_query()` is called, the calling environment isn't a call, so `match.call()` fails.

### What changes are included in this PR?

This PR wraps the `match.call()` call in a `try()`. The call is only used to do `abandon_ship()` on in-memory data anyway. So if the call is not available, it treats it like you're making a query on a Dataset and it tells you to `collect()` yourself. 

### Are these changes tested?

I couldn't figure out how to reproduce what was going on inside `tidyquery` to write a reproducer, and I don't think this is worth adding `tidyquery` to Suggests for. I confirmed locally that `tidyquery` tests pass with this change, so our revdeps should be clear.

### Are there any user-facing changes?

🙅 

Authored-by: Neal Richardson <neal.p.richardson@gmail.com>
Signed-off-by: Jonathan Keane <jkeane@gmail.com>
  • Loading branch information
nealrichardson authored and jonkeane committed Jul 31, 2024
1 parent cbd6b5c commit a7d24cd
Showing 1 changed file with 10 additions and 3 deletions.
13 changes: 10 additions & 3 deletions r/R/dplyr-eval.R
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,12 @@ try_arrow_dplyr <- function(expr) {
parent <- caller_env()
# Make sure that the call is available in the parent environment
# so that we can use it in abandon_ship, if needed
evalq(call <- match.call(), parent)
# (but don't error if we're in some weird context where we can't get the call,
# which could happen if you're code-generating or something?)
try(
evalq(call <- match.call(), parent),
silent = !getOption("arrow.debug", FALSE)
)

tryCatch(
eval(expr, parent),
Expand All @@ -217,7 +222,10 @@ try_arrow_dplyr <- function(expr) {
# and that the function being called also exists in the dplyr namespace.
abandon_ship <- function(err, env) {
.data <- get(".data", envir = env)
if (query_on_dataset(.data)) {
# If there's no call (see comment in try_arrow_dplyr), we can't eval with
# dplyr even if the data is in memory already
call <- try(get("call", envir = env), silent = TRUE)
if (query_on_dataset(.data) || inherits(call, "try-error")) {
# Add a note suggesting `collect()` to the error message.
# If there are other suggestions already there (with the > arrow name),
# collect() isn't the only suggestion, so message differently
Expand All @@ -231,7 +239,6 @@ abandon_ship <- function(err, env) {
}

# Else, warn, collect(), and run in regular dplyr
call <- get("call", envir = env)
rlang::warn(
message = paste0("In ", format_expr(err$call), ": "),
body = c("i" = conditionMessage(err), ">" = "Pulling data into R")
Expand Down

0 comments on commit a7d24cd

Please sign in to comment.