From 46a7da6d1f621addc960341ad8387611e15dd42f Mon Sep 17 00:00:00 2001 From: Toph Allen Date: Wed, 30 Apr 2025 13:38:01 -0400 Subject: [PATCH 1/3] minor improvements to time zone parsing --- R/parse.R | 34 ++++++++++----------------------- tests/testthat/test-parse.R | 38 ++++++++++++++++++++++++++++++++----- 2 files changed, 43 insertions(+), 29 deletions(-) diff --git a/R/parse.R b/R/parse.R index 367473b99..4c704450a 100644 --- a/R/parse.R +++ b/R/parse.R @@ -154,35 +154,21 @@ coerce_datetime <- function(x, to, ...) { # - "2020-01-01T00:02:03-01:00" # nolint end parse_connect_rfc3339 <- function(x) { - # Convert any timestamps with offsets to a format recognized by `strptime`. + # Convert timestamps with offsets to a format recognized by `strptime`. x <- gsub("([+-]\\d\\d):(\\d\\d)$", "\\1\\2", x) + x <- sub("Z$", "+0000", x) - # `purrr::map2_vec()` converts to POSIXct automatically, but we need - # `as.POSIXct()` in there to account vectors of length 1, which it seems are - # not converted. - # - # Parse with an inner call to `strptime()`; convert the resulting `POSIXlt` - # object to `POSIXct`. + # Parse with an inner call to `strptime()`, which returns a POSIXlt object, + # and convert that to `POSIXct`. # # We must specify `tz` in the inner call to correctly compute date math. - # Specifying `tz` when parsing just changes the time zone without doing any - # date math! + # Specifying `tz` when in the outer call just changes the time zone without + # doing any date math! # - # > xlt - # [1] "2024-08-29 16:36:33 EDT" - # > tzone(xlt) - # [1] "America/New_York" - # > as.POSIXct(xlt, tz = "UTC") - # [1] "2024-08-29 16:36:33 UTC" - purrr::map_vec(x, function(.x) { - # Times with and without offsets require different formats. - format_string <- ifelse( - grepl("Z$", .x), - "%Y-%m-%dT%H:%M:%OSZ", - "%Y-%m-%dT%H:%M:%OS%z" - ) - as.POSIXct(strptime(.x, format = format_string, tz = "UTC")) - }) + # > xlt [1] "2024-08-29 16:36:33 EDT" tzone(xlt) [1] "America/New_York" + # as.POSIXct(xlt, tz = "UTC") [1] "2024-08-29 16:36:33 UTC" + format_string <- "%Y-%m-%dT%H:%M:%OS%z" + as.POSIXct(x, format = format_string, tz = Sys.timezone()) } vec_cast.POSIXct.double <- # nolint: object_name_linter diff --git a/tests/testthat/test-parse.R b/tests/testthat/test-parse.R index 9d1b5459a..e1aef3a8d 100644 --- a/tests/testthat/test-parse.R +++ b/tests/testthat/test-parse.R @@ -47,7 +47,10 @@ test_that("coerce_datetime fills the void", { }) test_that("parse_connect_rfc3339() parses timestamps with offsets as expected", { - withr::defer(Sys.setenv(TZ = Sys.getenv("TZ"))) + original_tz <- Sys.getenv("TZ") + withr::defer({ + Sys.setenv(TZ = original_tz) + }) x_mixed <- c( "2023-08-22T14:13:14Z", @@ -75,16 +78,15 @@ test_that("parse_connect_rfc3339() parses timestamps with offsets as expected", single_offset <- "2023-08-22T15:13:14+01:00" + Sys.setenv(TZ = "America/New_York") expected <- as.POSIXct(strptime( c( "2023-08-22T14:13:14+0000", "2020-01-01T01:02:03+0000" ), format = "%Y-%m-%dT%H:%M:%S%z", - tz = "UTC" + tz = Sys.timezone() )) - - Sys.setenv(TZ = "America/New_York") expect_identical(parse_connect_rfc3339(x_mixed), rep(expected, 2)) expect_identical(parse_connect_rfc3339(x_zero_offset), expected) expect_identical(parse_connect_rfc3339(x_plus_one), expected) @@ -93,6 +95,14 @@ test_that("parse_connect_rfc3339() parses timestamps with offsets as expected", expect_identical(parse_connect_rfc3339(single_offset), expected[1]) Sys.setenv(TZ = "UTC") + expected <- as.POSIXct(strptime( + c( + "2023-08-22T14:13:14+0000", + "2020-01-01T01:02:03+0000" + ), + format = "%Y-%m-%dT%H:%M:%S%z", + tz = Sys.timezone() + )) expect_identical(parse_connect_rfc3339(x_mixed), rep(expected, 2)) expect_identical(parse_connect_rfc3339(x_zero_offset), expected) expect_identical(parse_connect_rfc3339(x_plus_one), expected) @@ -101,6 +111,14 @@ test_that("parse_connect_rfc3339() parses timestamps with offsets as expected", expect_identical(parse_connect_rfc3339(single_offset), expected[1]) Sys.setenv(TZ = "Asia/Tokyo") + expected <- as.POSIXct(strptime( + c( + "2023-08-22T14:13:14+0000", + "2020-01-01T01:02:03+0000" + ), + format = "%Y-%m-%dT%H:%M:%S%z", + tz = Sys.timezone() + )) expect_identical(parse_connect_rfc3339(x_mixed), rep(expected, 2)) expect_identical(parse_connect_rfc3339(x_zero_offset), expected) expect_identical(parse_connect_rfc3339(x_plus_one), expected) @@ -109,7 +127,14 @@ test_that("parse_connect_rfc3339() parses timestamps with offsets as expected", expect_identical(parse_connect_rfc3339(single_offset), expected[1]) }) + test_that("parse_connect_rfc3339() handles fractional seconds", { + original_tz <- Sys.getenv("TZ") + withr::defer({ + Sys.setenv(TZ = original_tz) + }) + + Sys.setenv(TZ = "UTC") expected <- as.POSIXct(strptime( c( "2024-12-06T19:09:29.948016766+0000", @@ -125,7 +150,10 @@ test_that("parse_connect_rfc3339() handles fractional seconds", { }) test_that("make_timestamp produces expected output", { - withr::defer(Sys.setenv(TZ = Sys.getenv("TZ"))) + original_tz <- Sys.getenv("TZ") + withr::defer({ + Sys.setenv(TZ = original_tz) + }) x_mixed <- c( "2023-08-22T14:13:14Z", From 41c8549616b361cfb2f48d8a4df8355fd3a3ac5b Mon Sep 17 00:00:00 2001 From: Toph Allen Date: Wed, 30 Apr 2025 13:44:50 -0400 Subject: [PATCH 2/3] news, consistency --- NEWS.md | 5 ++++- R/parse.R | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index 63018033b..02b591ce2 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,7 +2,10 @@ ## Enhancements and fixes -- `get_groups()` now paginates through all results when a `prefix` is provided, if the Connect server API version supports pagination. (#328) +- `get_groups()` now paginates through all results when a `prefix` is provided, + if the Connect server API version supports pagination. (#328) +- Timestamps from the Connect server are now displayed in your local time zone, + rather than in UTC. (#400) # connectapi 0.7.0 diff --git a/R/parse.R b/R/parse.R index 4c704450a..ed9c3d01b 100644 --- a/R/parse.R +++ b/R/parse.R @@ -156,7 +156,7 @@ coerce_datetime <- function(x, to, ...) { parse_connect_rfc3339 <- function(x) { # Convert timestamps with offsets to a format recognized by `strptime`. x <- gsub("([+-]\\d\\d):(\\d\\d)$", "\\1\\2", x) - x <- sub("Z$", "+0000", x) + x <- gsub("Z$", "+0000", x) # Parse with an inner call to `strptime()`, which returns a POSIXlt object, # and convert that to `POSIXct`. From 7df3126198a54c3e92f12e7f26415df9166c5c93 Mon Sep 17 00:00:00 2001 From: Toph Allen Date: Wed, 30 Apr 2025 15:11:57 -0400 Subject: [PATCH 3/3] use withr::local_envvar --- tests/testthat/test-parse.R | 29 +++++++---------------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/tests/testthat/test-parse.R b/tests/testthat/test-parse.R index e1aef3a8d..4e182704b 100644 --- a/tests/testthat/test-parse.R +++ b/tests/testthat/test-parse.R @@ -47,11 +47,6 @@ test_that("coerce_datetime fills the void", { }) test_that("parse_connect_rfc3339() parses timestamps with offsets as expected", { - original_tz <- Sys.getenv("TZ") - withr::defer({ - Sys.setenv(TZ = original_tz) - }) - x_mixed <- c( "2023-08-22T14:13:14Z", "2020-01-01T01:02:03Z", @@ -78,7 +73,7 @@ test_that("parse_connect_rfc3339() parses timestamps with offsets as expected", single_offset <- "2023-08-22T15:13:14+01:00" - Sys.setenv(TZ = "America/New_York") + withr::local_envvar(TZ = "America/New_York") expected <- as.POSIXct(strptime( c( "2023-08-22T14:13:14+0000", @@ -94,7 +89,7 @@ test_that("parse_connect_rfc3339() parses timestamps with offsets as expected", expect_identical(parse_connect_rfc3339(single_zero_offset), expected[1]) expect_identical(parse_connect_rfc3339(single_offset), expected[1]) - Sys.setenv(TZ = "UTC") + withr::local_envvar(TZ = "UTC") expected <- as.POSIXct(strptime( c( "2023-08-22T14:13:14+0000", @@ -110,7 +105,7 @@ test_that("parse_connect_rfc3339() parses timestamps with offsets as expected", expect_identical(parse_connect_rfc3339(single_zero_offset), expected[1]) expect_identical(parse_connect_rfc3339(single_offset), expected[1]) - Sys.setenv(TZ = "Asia/Tokyo") + withr::local_envvar(TZ = "Asia/Tokyo") expected <- as.POSIXct(strptime( c( "2023-08-22T14:13:14+0000", @@ -129,12 +124,7 @@ test_that("parse_connect_rfc3339() parses timestamps with offsets as expected", test_that("parse_connect_rfc3339() handles fractional seconds", { - original_tz <- Sys.getenv("TZ") - withr::defer({ - Sys.setenv(TZ = original_tz) - }) - - Sys.setenv(TZ = "UTC") + withr::local_envvar(TZ = "UTC") expected <- as.POSIXct(strptime( c( "2024-12-06T19:09:29.948016766+0000", @@ -150,11 +140,6 @@ test_that("parse_connect_rfc3339() handles fractional seconds", { }) test_that("make_timestamp produces expected output", { - original_tz <- Sys.getenv("TZ") - withr::defer({ - Sys.setenv(TZ = original_tz) - }) - x_mixed <- c( "2023-08-22T14:13:14Z", "2020-01-01T01:02:03Z", @@ -186,7 +171,7 @@ test_that("make_timestamp produces expected output", { "2020-01-01T01:02:03Z" ) - Sys.setenv(TZ = "America/New_York") + withr::local_envvar(TZ = "America/New_York") expect_equal( make_timestamp(coerce_datetime(x_mixed, NA_datetime_)), rep(outcome, 2) @@ -213,7 +198,7 @@ test_that("make_timestamp produces expected output", { ) expect_equal(make_timestamp(outcome), outcome) - Sys.setenv(TZ = "UTC") + withr::local_envvar(TZ = "UTC") expect_equal( make_timestamp(coerce_datetime(x_mixed, NA_datetime_)), rep(outcome, 2) @@ -240,7 +225,7 @@ test_that("make_timestamp produces expected output", { ) expect_equal(make_timestamp(outcome), outcome) - Sys.setenv(TZ = "Asia/Tokyo") + withr::local_envvar(TZ = "Asia/Tokyo") expect_equal( make_timestamp(coerce_datetime(x_mixed, NA_datetime_)), rep(outcome, 2)