[GitHub] [arrow] jonkeane commented on pull request #12357: ARROW-14817 [R] Implement bindings for lubridate::tz

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

jonkeane commented on pull request #12357:
URL: https://github.com/apache/arrow/pull/12357#issuecomment-1034947201


   I'm going to answer these two points below together, since they are similar. Both have tradeoffs, but what I do in circumstances like this is to look through our other bindings and see if I can find examples that do similar things.
   
   > To add to the conversation above. I think the right way to go is to extract timezone from the timestamp metadata and not look at each individual value. However, that could in a situation where a value is NA
   
   I agree that we should use the column metadata here (which is what it looks like you're doing), but then wrap it in an if_else detection if any elements are `NA` (one example is how we implemented `if_else()`(😂) https://github.com/apache/arrow/blob/af33dd1157eb8d7d9bfac25ebf61445b793b7943/r/R/dplyr-funcs-conditional.R#L51-L61 we also do similar with `is.finite()` and `is.infinite()` https://github.com/apache/arrow/blob/af33dd1157eb8d7d9bfac25ebf61445b793b7943/r/R/dplyr-funcs-type.R#L239-L249
   
   
   > I can't figure out how to surface an error from tz() as it gets caught by the dplyr query and then the dplyr-equivalent expression still gets evaluated in R.
   
   One example of having an error in a binding is https://github.com/apache/arrow/blob/af33dd1157eb8d7d9bfac25ebf61445b793b7943/r/R/dplyr-funcs-conditional.R#L86-L88 


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