jonkeane commented on a change in pull request #11105:
URL: https://github.com/apache/arrow/pull/11105#discussion_r712268519
##########
File path: r/R/dplyr-functions.R
##########
@@ -672,6 +672,18 @@ nse_funcs$strptime <- function(x, format = "%Y-%m-%d
%H:%M:%S", tz = NULL, unit
Expression$create("strptime", x, options = list(format = format, unit =
unit))
}
+nse_funcs$strftime <- function(x, format = "", tz = "", usetz = FALSE) {
+ if (usetz) {
+ format <- paste(format, "%Z")
+ }
+ if (tz == "") {
+ tz <- Sys.timezone()
+ }
+ unit <- TimestampType__unit(x$type())
+ ts <- Expression$create("cast", x, options = list(to_type = timestamp(unit,
tz)))
Review comment:
Ah yes, I think I see what's going on here: when you're accessing
`unit`, we already have some helper methods on the type class so you don't need
to call `TimestampType__unit()` directly. What you want for line 682 instead if
something like `unit <- x$type$unit()`.
I'm a little surprised/confused by using `type()` in the code that's here
instead of `type`, so it's _possible_ that at this point in evaluation you'll
actually need `unit <- x$type()$unit()`, but I haven't been able to construct
that in my testing:
```
> library(arrow)
> library(tibble)
>
> times <- tibble(x = c(lubridate::ymd_hms("2018-10-07 19:04:05"), NA))
> tab <- Table$create(times)
>
> tab$x$type$unit()
[1] 2
> tab$x$type()$unit()
Error: attempt to apply non-function
```
--
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]
