Skip to content

Commit

Permalink
ARROW-14442: [R] fix behaviour when converting timestamps with "" as …
Browse files Browse the repository at this point in the history
…tzone

Closes #12240 from dragosmg/timestampts_missing_timezone

Authored-by: Dragoș Moldovan-Grünfeld <[email protected]>
Signed-off-by: Jonathan Keane <[email protected]>
  • Loading branch information
dragosmg authored and jonkeane committed Apr 11, 2022
1 parent e453ffe commit 633687c
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 9 deletions.
6 changes: 4 additions & 2 deletions r/src/type_infer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ std::shared_ptr<arrow::DataType> InferArrowTypeFromVector<INTSXP>(SEXP x) {
} else if (Rf_inherits(x, "POSIXct")) {
auto tzone_sexp = Rf_getAttrib(x, symbols::tzone);
if (Rf_isNull(tzone_sexp)) {
return timestamp(TimeUnit::MICRO);
auto systzone_sexp = cpp11::package("base")["Sys.timezone"];
return timestamp(TimeUnit::MICRO, CHAR(STRING_ELT(systzone_sexp(), 0)));
} else {
return timestamp(TimeUnit::MICRO, CHAR(STRING_ELT(tzone_sexp, 0)));
}
Expand All @@ -88,7 +89,8 @@ std::shared_ptr<arrow::DataType> InferArrowTypeFromVector<REALSXP>(SEXP x) {
if (Rf_inherits(x, "POSIXct")) {
auto tzone_sexp = Rf_getAttrib(x, symbols::tzone);
if (Rf_isNull(tzone_sexp)) {
return timestamp(TimeUnit::MICRO);
auto systzone_sexp = cpp11::package("base")["Sys.timezone"];
return timestamp(TimeUnit::MICRO, CHAR(STRING_ELT(systzone_sexp(), 0)));
} else {
return timestamp(TimeUnit::MICRO, CHAR(STRING_ELT(tzone_sexp, 0)));
}
Expand Down
27 changes: 24 additions & 3 deletions r/tests/testthat/test-Array.R
Original file line number Diff line number Diff line change
Expand Up @@ -260,18 +260,39 @@ test_that("array supports POSIXct (ARROW-3340)", {
expect_array_roundtrip(times2, timestamp("us", "US/Eastern"))
})

test_that("array supports POSIXct without timezone", {
# Make sure timezone is not set
test_that("array uses local timezone for POSIXct without timezone", {
withr::with_envvar(c(TZ = ""), {
times <- strptime("2019-02-03 12:34:56", format = "%Y-%m-%d %H:%M:%S") + 1:10
expect_array_roundtrip(times, timestamp("us", ""))
expect_equal(attr(times, "tzone"), NULL)
expect_array_roundtrip(times, timestamp("us", Sys.timezone()))

# Also test the INTSXP code path
skip("Ingest_POSIXct only implemented for REALSXP")
times_int <- as.integer(times)
attributes(times_int) <- attributes(times)
expect_array_roundtrip(times_int, timestamp("us", ""))
})

# If there is a timezone set, we record that
withr::with_timezone("Pacific/Marquesas", {
times <- strptime("2019-02-03 12:34:56", format = "%Y-%m-%d %H:%M:%S") + 1:10
expect_equal(attr(times, "tzone"), "Pacific/Marquesas")
expect_array_roundtrip(times, timestamp("us", "Pacific/Marquesas"))

times_with_tz <- strptime(
"2019-02-03 12:34:56",
format = "%Y-%m-%d %H:%M:%S",
tz = "Asia/Katmandu") + 1:10
expect_equal(attr(times, "tzone"), "Asia/Katmandu")
expect_array_roundtrip(times, timestamp("us", "Asia/Katmandu"))
})

# and although the TZ is NULL in R, we set it to the Sys.timezone()
withr::with_timezone(NA, {
times <- strptime("2019-02-03 12:34:56", format = "%Y-%m-%d %H:%M:%S") + 1:10
expect_equal(attr(times, "tzone"), NULL)
expect_array_roundtrip(times, timestamp("us", Sys.timezone()))
})
})

test_that("Timezone handling in Arrow roundtrip (ARROW-3543)", {
Expand Down
4 changes: 0 additions & 4 deletions r/tests/testthat/test-dplyr-funcs-datetime.R
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,6 @@ test_that("extract yday from date", {
})

test_that("leap_year mirror lubridate", {

compare_dplyr_binding(
.input %>%
mutate(x = leap_year(date)) %>%
Expand Down Expand Up @@ -721,7 +720,6 @@ test_that("leap_year mirror lubridate", {
))
)
)

})

test_that("am/pm mirror lubridate", {
Expand All @@ -741,10 +739,8 @@ test_that("am/pm mirror lubridate", {
),
format = "%Y-%m-%d %H:%M:%S"
)

)
)

})

test_that("extract tz", {
Expand Down

0 comments on commit 633687c

Please sign in to comment.