jonkeane commented on a change in pull request #12433:
URL: https://github.com/apache/arrow/pull/12433#discussion_r813328095



##########
File path: r/R/dplyr-funcs-datetime.R
##########
@@ -164,4 +164,10 @@ register_bindings_datetime <- function() {
       return(semester)
     }
   })
+  register_binding("date", function(x) {
+    if (inherits(x, "POSIXct")) {
+      x <- vec_to_Array(x, date32())
+    }
+    x$cast(date32())

Review comment:
       This implementation made me think of the various `as.*` methods we have 
defined [1] (since this is similar to `as.Date()`). Which all use a simpler 
setup to create a cast operation. However, I noticed that for those, they are 
using `Expression$create(...)` rather than the `build_expr(...)` helper [2]. 
That `build_expr(...)` here _should_ handle the wrapping of R objects into 
Scalars that you're doing in this first bit here. Can you try that out and 
confirm that it does what we are looking for with inputs that are both columns 
in an arrow table/recordbatch/dataset etc. as well as if it's a single value 
from R (i.e. `mutate(term_start = date("2020-01-01")`). 
   
   Separately: We should also open a jira if one doesn't exist to clean up 
those `as.*` methods to use `build_expr()`
   
   
   [1] 
https://github.com/apache/arrow/blob/a916e6015d7cab397d3b769d760b943a60ea3d97/r/R/dplyr-funcs-type.R#L45-L78
   [2] 
https://github.com/apache/arrow/blob/a916e6015d7cab397d3b769d760b943a60ea3d97/r/R/expression.R#L195-L259




-- 
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-unsubscr...@arrow.apache.org

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


Reply via email to