ARROW-16415: [R] Update strptime binding signature with the tz argument

[dragosmg]

This PR enables the tz argument for the strptime binding by using the assume_timezone compute function.

Current behaviour:

library(arrow, warn.conflicts=FALSE)
library(dplyr, warn.conflicts=FALSE)
test_df<-tibble::tibble(
timestamp_naive_string= c("2018-10-07 19:04:05", NA)
)
test_df %>% arrow_table() %>% mutate(
timestamp= strptime(
timestamp_naive_string, format="%Y-%m-%d %H:%M:%S", tz="Pacific/Marquesas"
)
) %>%
collect()
#> Warning: In strptime(timestamp_naive_string, format = "%Y-%m-%d %H:%M:%S", ...,#> Time zone argument not supported in Arrow; pulling data into R#> # A tibble: 2 × 2#> timestamp_naive_string timestamp #> <chr> <dttm> #> 1 2018-10-07 19:04:05 2018-10-07 19:04:05#> 2 <NA> NA

^{Created on 2022-05-18 by the reprex package (v2.0.1)}

Future/desired behaviour:

library(arrow, warn.conflicts=FALSE)
library(dplyr, warn.conflicts=FALSE)
test_df<-tibble::tibble(
timestamp_naive_string= c("2018-10-07 19:04:05", NA)
)
a<-test_df %>% arrow_table() %>% mutate(
timestamp= strptime(
timestamp_naive_string, format="%Y-%m-%d %H:%M:%S", tz="Pacific/Marquesas"
)
) %>%
collect()
a#> # A tibble: 2 × 2#> timestamp_naive_string timestamp #> <chr> <dttm> #> 1 2018-10-07 19:04:05 2018-10-07 19:04:05#> 2 <NA> NA
attributes(a$timestamp)
#> $class#> [1] "POSIXlt" "POSIXt" #> #> $tzone#> [1] "Pacific/Marquesas"

^{Created on 2022-05-19 by the reprex package (v2.0.1)}

[github-actions]

https://issues.apache.org/jira/browse/ARROW-16415

[thisisnic]

I think you might have some issues where the default argument of tz in strptime() in R is different to the one here. I tried running this test I wrote, but got an error:

> t_string %>%
+ arrow_table() %>%
+ mutate(
+ x = strptime(x, format = "%Y-%m-%d %H:%M:%S", tz = "")
+ ) %>%
+ collect() -> out
Error in `collect()`:
! Invalid: Cannot locate timezone '': not found in timezone database
/home/nic2/arrow/cpp/src/arrow/compute/kernels/scalar_temporal_unary.cc:128 LocateZone(options.timezone)
/home/nic2/arrow/cpp/src/arrow/compute/exec.cc:701 kernel_->exec(kernel_ctx_, batch, &out)
/home/nic2/arrow/cpp/src/arrow/compute/exec.cc:642 ExecuteBatch(batch, listener)
/home/nic2/arrow/cpp/src/arrow/compute/exec/expression.cc:597 executor->Execute(arguments, &listener)
/home/nic2/arrow/cpp/src/arrow/compute/exec/project_node.cc:90 ExecuteScalarExpression(simplified_expr, target, plan()->exec_context())
/home/nic2/arrow/cpp/src/arrow/compute/exec/exec_plan.cc:467 iterator_.Next()
/home/nic2/arrow/cpp/src/arrow/record_batch.cc:337 ReadNext(&batch)
/home/nic2/arrow/cpp/src/arrow/record_batch.cc:351 ToRecordBatches()

[thisisnic]

Other than the comment it'd be great to see added, I think this is good to go. @jonkeane - mind taking a look when you have a chance, in case I've missed anything?

[jonkeane]

This is looking good — a comment about an additional test + a few more comments that would be helpful

[jonkeane]

Looks good, thanks for sticking with it