[GitHub] [arrow] nealrichardson commented on a change in pull request #11156: ARROW-13777: [R] mutate after group_by should be ok as long as there are only scalar functions

[GitBox <gi...@apache.org>]

nealrichardson commented on a change in pull request #11156:
URL: https://github.com/apache/arrow/pull/11156#discussion_r708595599



##########
File path: r/R/util.R
##########
@@ -58,10 +58,22 @@ r_symbolic_constants <- c(
   "NA_integer_", "NA_real_", "NA_complex_", "NA_character_"
 )
 
+is_function <- function(expr, name) {
+  if (!is.call(expr)) {
+    return(FALSE)
+  } else {
+    if (deparse(expr[[1]]) == name) {
+      return(TRUE)
+    }
+    out <- lapply(expr, is_function, name)
+  }
+  any(vapply(out, isTRUE, TRUE))
+}
+
 all_funs <- function(expr) {
   # Don't use setdiff so that we preserve duplicates

Review comment:
       ```suggestion
     # Don't just setdiff(all.names, all.vars) because that would drop functions that share a name with a var
   ```
   

##########
File path: r/R/util.R
##########
@@ -58,10 +58,22 @@ r_symbolic_constants <- c(
   "NA_integer_", "NA_real_", "NA_complex_", "NA_character_"
 )
 
+is_function <- function(expr, name) {
+  if (!is.call(expr)) {
+    return(FALSE)
+  } else {
+    if (deparse(expr[[1]]) == name) {
+      return(TRUE)
+    }
+    out <- lapply(expr, is_function, name)
+  }
+  any(vapply(out, isTRUE, TRUE))

Review comment:
       ```suggestion
     any(map_lgl(out, isTRUE))
   ```

##########
File path: r/R/util.R
##########
@@ -58,10 +58,22 @@ r_symbolic_constants <- c(
   "NA_integer_", "NA_real_", "NA_complex_", "NA_character_"
 )
 
+is_function <- function(expr, name) {
+  if (!is.call(expr)) {
+    return(FALSE)
+  } else {
+    if (deparse(expr[[1]]) == name) {
+      return(TRUE)
+    }
+    out <- lapply(expr, is_function, name)
+  }
+  any(vapply(out, isTRUE, TRUE))
+}
+
 all_funs <- function(expr) {
   # Don't use setdiff so that we preserve duplicates
-  out <- all.names(expr)
-  out[!(out %in% all.vars(expr))]
+  names <- all.names(expr)
+  names[vapply(names, function(name) {is_function(expr, name)}, TRUE)]

Review comment:
       Let's modernize this since we use purrr elsewhere. (Could even make it a one-liner with [keep](https://purrr.tidyverse.org/reference/keep.html) if you wanted)
   
   ```suggestion
     names <- all_names(expr)
     names[map_lgl(names, ~ is_function(expr, .))]
   ```




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org