larry77 commented on PR #49256:
URL: https://github.com/apache/arrow/pull/49256#issuecomment-3897747465

   > Thanks for making this PR @larry77! This is looking good, a few notes from 
me:
   > 
   >     * Would you mind adding `filter_out` to `supported_dplyr_methods` in 
`arrow/r/R/arrow-package.R`?
   > 
   >     * Tests look good; I went to see if the core dplyr ones did much 
different but I think those cases you included cover everything.
   > 
   >     * The code in the body of `filter_out()` appears to be copied from 
`filter()`; could we instead extract out that code into its own function and 
then use it in both?
   
   Thanks for the review!
   
   – Added filter_out to supported_dplyr_methods in r/R/arrow-package.R.
   – Refactored filter() and filter_out() to share a common implementation (no 
duplicated body).
   
   Local tests pass aside from the existing locale-related .cache flake in the 
“missing column” test, which seems environment-specific.


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

Reply via email to