AlenkaF commented on a change in pull request #10334:
URL: https://github.com/apache/arrow/pull/10334#discussion_r641408098
##########
File path: r/tests/testthat/test-dplyr-string-functions.R
##########
@@ -493,3 +493,81 @@ test_that("edge cases in string detection and
replacement", {
tibble(x = c("ABC"))
)
})
+
+test_that("strptime", {
+
+ t_string <- tibble(x = c("2018-10-07 19:04:05", NA))
+ t_stamp <- tibble(x = c(lubridate::ymd_hms("2018-10-07 19:04:05"), NA))
+ t_stampPDT <- tibble(x = c(lubridate::ymd_hms("2018-10-07 19:04:05", tz =
"PDT"), NA))
Review comment:
I missed a typo in code, line 554: `t_stampPDT` should be used instead
of `t_stamp`.
But it relates to your [comment
1](https://github.com/apache/arrow/pull/10334/files#r640822991) and [comment
2](https://github.com/apache/arrow/pull/10334#pullrequestreview-670405891).
I added example `t_stampPDT` to the test to see if I get a warning as `tz`
agrument is given. I do but then data is pulled into `R`. Test now correctly
fails as `lubridate` converts time to match PDT time zone. But then it should
`stop()` as Neal
[suggested](https://github.com/apache/arrow/pull/10334#discussion_r635544090)
but I am not sure I know how to do that.
Adding separate test to check if we error correctly could be something in
the lines of:
```
test_that("errors in strptime", {
# Error when tz is passed
x <- Expression$field_ref("x")
expect_error(
nse_funcs$strptime(x, tz = "PDT"),
'Time zone argument not supported by Arrow'
)
})
```
and then lines from
[comment](https://github.com/apache/arrow/pull/10334/files#r640822991) are
redundant.
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
