Copilot commented on code in PR #51291:
URL: https://github.com/apache/arrow/pull/51291#discussion_r3981796424
##########
r/R/dplyr-eval.R:
##########
@@ -128,6 +132,30 @@ add_user_functions_to_mask <- function(expr, mask) {
invisible()
}
+add_user_variables_to_mask <- function(expr, mask) {
+ # The function bindings environment sits between the columns and the user's
+ # environment in the mask, so a symbol like `date` in `filter(Date == date)`
+ # would resolve to the `date()` binding rather than the user's variable.
+ # dplyr would find the variable, so bind it into the mask so we do too.
+ if (is_quosure(expr)) {
+ function_env <- parent.env(parent.env(mask))
+ quo_env <- quo_get_env(expr)
+ # all.vars() returns symbols that aren't in function position
+ vars_in_expr <- all.vars(quo_get_expr(expr))
+ columns <- names(mask$.data)
+ shadowed <- setdiff(intersect(vars_in_expr, ls(function_env, all.names =
TRUE)), columns)
+ for (var_name in shadowed) {
+ user_var <- get0(var_name, quo_env)
+ # Functions from the user's environment (like lubridate::day) shouldn't
+ # take precedence over the bindings
+ if (!is.null(user_var) && !is.function(user_var)) {
Review Comment:
`get0(var_name, quo_env)` defaults to `inherits = FALSE`, so variables
defined in parent environments of the quosure env (common with lexical scoping
inside functions) won’t be found and therefore won’t shadow the function
binding. This can still diverge from dplyr’s name resolution in those cases.
--
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]
