Remove timezones no longer supported by Debian (#584)

* Remove timezones no longer supported by Debian (fix #582) * Warn about `sas_numeric_to_date()` timezone usage * Update test for new version of tidygraph * Clarify SAS warning Co-authored-by: Sam Firke <[email protected]> * Ensure that `tz` is respected in `sas_numeric_to_date()`; test update for recent change; clarify minor breaking change for `sas_numeric_to_date()` --------- Co-authored-by: Sam Firke <[email protected]>
sfirke · Nov 19, 2024 · 74345da · 74345da
1 parent 709b2ab
commit 74345da
Show file tree

Hide file tree

Showing 8 changed files with 35 additions and 21 deletions.
diff --git a/DESCRIPTION b/DESCRIPTION
@@ -52,4 +52,4 @@ Config/testthat/edition: 3
 Encoding: UTF-8
 LazyData: true
 Roxygen: list(markdown = TRUE)
-RoxygenNote: 7.3.1
+RoxygenNote: 7.3.2
diff --git a/NEWS.md b/NEWS.md
@@ -8,6 +8,7 @@ These are all minor breaking changes resulting from enhancements and are not exp
 
 * When `tabyl()` is called on a data.frame containing labels, it now displays the label attribute as the name of the first column in the the resulting `tabyl` object (@olivroy, #394). This may break subsequent code that refers to the output of such a `tabyl` by column name. To maintain the previous behavior of ignoring variable labels, you can remove the labels with a function like `haven::zap_labels()` or `labelled::remove_labels()` before calling `tabyl()`.
 
+* `sas_numeric_to_date()` now warns for timezones other than "UTC" due to the way that SAS loads timezones, and the default timezone for `sas_numeric_to_date()` is now "UTC" instead of "" (#583, @billdenney)
 
 ## New features
 
@@ -31,7 +32,7 @@ These are all minor breaking changes resulting from enhancements and are not exp
 
 * Remove dplyr verbs superseded in dplyr 1.0.0 (#547, @olivroy)
 
-* Restyle the package and vignettes according to the [tidyverse style guide](https://style.tidyverse.org) (#548, olivroy)
+* Restyle the package and vignettes according to the [tidyverse style guide](https://style.tidyverse.org) (#548, @olivroy)
 
 # janitor 2.2.0 (2023-02-02)
 

diff --git a/R/sas_dates.R b/R/sas_dates.R
@@ -16,11 +16,16 @@
 #' sas_numeric_to_date(time_num = 3600) # 01:00:00
 #' @family date-time cleaning
 #' @export
-sas_numeric_to_date <- function(date_num, datetime_num, time_num, tz = "") {
+sas_numeric_to_date <- function(date_num, datetime_num, time_num, tz = "UTC") {
   # Confirm that a usable set of input arguments is given
   has_date <- !missing(date_num)
   has_datetime <- !missing(datetime_num)
   has_time <- !missing(time_num)
+  stopifnot(is.character(tz))
+  stopifnot(length(tz) == 1)
+  if (tz != "UTC") {
+    warning("SAS may not properly store timezones other than UTC. Consider confirming the accuracy of the resulting data.")
+  }
   if (has_date & has_datetime) {
     stop("Must not give both `date_num` and `datetime_num`")
   } else if (has_time & has_datetime) {
@@ -37,8 +42,10 @@ sas_numeric_to_date <- function(date_num, datetime_num, time_num, tz = "") {
     if (!all(mask_na_match)) {
       stop("The same values are not NA for both `date_num` and `time_num`")
     }
-    ret <- as.POSIXct(86400 * date_num + time_num, origin = "1960-01-01", tz = tz)
-  } else if (has_datetime) {
+    datetime_num <- 86400 * date_num + time_num
+    has_datetime <- TRUE
+  }
+  if (has_datetime) {
     ret <- as.POSIXct(datetime_num, origin = "1960-01-01", tz = tz)
   } else if (has_date) {
     ret <- as.Date(date_num, origin = "1960-01-01")

diff --git a/man/sas_numeric_to_date.Rd b/man/sas_numeric_to_date.Rd
diff --git a/tests/testthat/test-clean-names.R b/tests/testthat/test-clean-names.R
@@ -595,7 +595,7 @@ test_that("tbl_graph/tidygraph", {
   skip_if_not_installed("tidygraph")
   # create test graph to test clean_names
   test_graph <-
-    tidygraph::play_erdos_renyi(10, 0.5) %>%
+    tidygraph::play_gnp(10, 0.5) %>%
     # create nodes wi
     tidygraph::bind_nodes(test_df) %>%
     dplyr::mutate(dplyr::across(dplyr::where(is.numeric), ~ dplyr::coalesce(x, 1)))

diff --git a/tests/testthat/test-convert_to_date.R b/tests/testthat/test-convert_to_date.R
@@ -48,8 +48,8 @@ test_that("convert_datetime works", {
     as.POSIXct("2009-07-06 12:13:14", tz = "UTC")
   )
   expect_equal(
-    convert_to_datetime("2009-07-06 12:13:14", tz = "EST"),
-    as.POSIXct("2009-07-06 12:13:14", tz = "EST"),
+    convert_to_datetime("2009-07-06 12:13:14", tz = "Etc/GMT-5"),
+    as.POSIXct("2009-07-06 12:13:14", tz = "Etc/GMT-5"),
     info = "The tz argument is respected"
   )
   expect_equal(
@@ -61,8 +61,8 @@ test_that("convert_datetime works", {
     as.POSIXct("2009-07-06 02:24", tz = "UTC")
   )
   expect_equal(
-    convert_to_datetime(40000.1, tz = "EST"),
-    as.POSIXct("2009-07-06 02:24", tz = "EST")
+    convert_to_datetime(40000.1, tz = "Etc/GMT-5"),
+    as.POSIXct("2009-07-06 02:24", tz = "Etc/GMT-5")
   )
   expect_equal(
     convert_to_datetime("40000"),
@@ -73,8 +73,8 @@ test_that("convert_datetime works", {
     as.POSIXct("2009-07-06 02:24", tz = "UTC")
   )
   expect_equal(
-    convert_to_datetime("40000.1", tz = "EST"),
-    as.POSIXct("2009-07-06 02:24", tz = "EST")
+    convert_to_datetime("40000.1", tz = "Etc/GMT-5"),
+    as.POSIXct("2009-07-06 02:24", tz = "Etc/GMT-5")
   )
   expect_equal(
     convert_to_datetime(factor("40000.1")),

diff --git a/tests/testthat/test-excel_time_to_numeric.R b/tests/testthat/test-excel_time_to_numeric.R
@@ -13,20 +13,20 @@ test_that("excel_time_to_numeric POSIX objects extract the correct part of the t
 })
 
 test_that("excel_time_to_numeric POSIX objects ignore the time zone", {
-  expect_equal(excel_time_to_numeric(as.POSIXct("1899-12-31 13:00", tz = "EST")), 13 * 3600)
+  expect_equal(excel_time_to_numeric(as.POSIXct("1899-12-31 13:00", tz = "Etc/GMT-5")), 13 * 3600)
   expect_equal(excel_time_to_numeric(as.POSIXct("1899-12-31 13:00", tz = "UTC")), 13 * 3600)
   expect_equal(
     excel_time_to_numeric(
-      as.POSIXct(c("1899-12-31 13:00", "1899-12-31 13:00"), tz = "EST")
+      as.POSIXct(c("1899-12-31 13:00", "1899-12-31 13:00"), tz = "Etc/GMT-5")
     ),
     rep(13 * 3600, 2)
   )
 })
 
 test_that("excel_time_to_numeric POSIXlt works like POSIXct", {
   expect_equal(
-    excel_time_to_numeric(as.POSIXct("1899-12-31 13:00", tz = "EST")),
-    excel_time_to_numeric(as.POSIXlt("1899-12-31 13:00", tz = "EST"))
+    excel_time_to_numeric(as.POSIXct("1899-12-31 13:00", tz = "Etc/GMT-5")),
+    excel_time_to_numeric(as.POSIXlt("1899-12-31 13:00", tz = "Etc/GMT-5"))
   )
 })
 

diff --git a/tests/testthat/test-sas_dates.R b/tests/testthat/test-sas_dates.R
@@ -17,12 +17,18 @@ test_that("sas_numeric_to_date", {
   )
   # NA management
   expect_equal(
-    sas_numeric_to_date(date_num = c(NA, 1), time_num = c(NA, 1), tz = "EST"),
-    as.POSIXct(c(NA, "1960-01-01 19:00:01"), tz = "EST")
+    sas_numeric_to_date(date_num = c(NA, 1), time_num = c(NA, 1), tz = "UTC"),
+    as.POSIXct(c(NA, "1960-01-02 00:00:01"), tz = "UTC")
   )
   expect_equal(
-    sas_numeric_to_date(date_num = NA, time_num = NA, tz = "EST"),
-    as.POSIXct(NA, tz = "EST")
+    sas_numeric_to_date(date_num = NA, time_num = NA, tz = "UTC"),
+    as.POSIXct(NA, tz = "UTC")
+  )
+  # Timezone warning (#583)
+  expect_warning(
+    sas_numeric_to_date(date_num = 1, time_num = 1, tz = "America/New_York"),
+    regexp = "SAS may not properly store timezones other than UTC. Consider confirming the accuracy of the resulting data.",
+    fixed = TRUE
   )
 })