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



##########
File path: r/R/dplyr-funcs-datetime.R
##########
@@ -31,7 +31,7 @@ register_bindings_datetime <- function() {
 
     unit <- make_valid_time_unit(unit, c(valid_time64_units, 
valid_time32_units))
 
-    Expression$create("strptime", x, options = list(format = format, unit = 
unit))
+    build_expr("strptime", x, options = list(format = format, unit = unit, 
error_is_null = TRUE))
   })

Review comment:
       Are you thinking of exposing it as an argument in our `strptime` 
binding? 
   
   We probably don't need that, since someone could do somethign like 
`arrow_strptime(..., error_is_null = TRUE)` to get that behavior. If we were to 
expose this to end users, I would recommend doing it via an option, so that all 
of the various places that use `strptime` in our bindings would all work the 
same — though even that I'm a bit doubtful someone will want to control. So I 
would say let's wait for someone to ask for it before implementing it




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