[GitHub] [arrow] jonkeane commented on a change in pull request #12402: ARROW-15659 [R] strptime should return NA (not error) with format mismatch

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

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

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