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



##########
File path: r/R/dplyr.R
##########
@@ -216,6 +215,52 @@ column_select <- function(.data, ..., .FUN = vars_select) {
   .data
 }
 
+relocate.arrow_dplyr_query <- function(.data, ..., .before = NULL, .after = 
NULL) {
+  .data <- arrow_dplyr_query(.data)
+
+  # TODO: look for unsupported tidyselect selection helpers in exprs(...),
+  # .before, and .after and throw an error if detected

Review comment:
       What is unsupported? 

##########
File path: r/R/dplyr.R
##########
@@ -216,6 +215,52 @@ column_select <- function(.data, ..., .FUN = vars_select) {
   .data
 }
 
+relocate.arrow_dplyr_query <- function(.data, ..., .before = NULL, .after = 
NULL) {

Review comment:
       Yes, here is fine I think, assuming that you're adapting and not just 
copying. If it's a significant enough copy, then we may need to add a note to 
the LICENSE file (IDK what the threshold is).
   
   Side note: I believe if `[<-` were defined for `arrow_dplyr_query`, then 
`relocate.arrow_dplyr_query <- dplyr:::relocate.data.frame` may just work (in 
which case we would want to make a PR to make that `relocate.default`). Then we 
wouldn't need to copy the code. It might be worth implementing that method, at 
least enough for column permutation (which is just operating on 
`$selected_columns`).




-- 
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.

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


Reply via email to