From 8f1c1d5a27ee7fdd1a3934c567d6a57959ec9684 Mon Sep 17 00:00:00 2001 From: Ben Letham Date: Wed, 3 Mar 2021 14:35:36 -0800 Subject: [PATCH] Fix bug in timezone handling in R (#1501) --- R/DESCRIPTION | 1 + R/R/prophet.R | 25 ++++++++++++++++--------- R/tests/testthat/test_prophet.R | 6 +++--- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/R/DESCRIPTION b/R/DESCRIPTION index 777c135..6f9f0f7 100644 --- a/R/DESCRIPTION +++ b/R/DESCRIPTION @@ -24,6 +24,7 @@ Imports: extraDistr, ggplot2, grid, + lubridate, methods, RcppParallel (>= 5.0.1), rstan (>= 2.18.1), diff --git a/R/R/prophet.R b/R/R/prophet.R index f8e249e..19436a3 100644 --- a/R/R/prophet.R +++ b/R/R/prophet.R @@ -240,15 +240,15 @@ validate_column_name <- function( #' Convert date vector #' -#' Convert the date to POSIXct object +#' Convert the date to POSIXct object. Dates without a specified timezone will +#' be given system time zone. Dates with specified timezone will keep it. #' -#' @param ds Date vector, can be consisted of characters -#' @param tz string time zone +#' @param ds Date vector #' #' @return vector of POSIXct object converted from date #' #' @keywords internal -set_date <- function(ds = NULL, tz = "GMT") { +set_date <- function(ds) { if (length(ds) == 0) { return(NULL) } @@ -257,12 +257,20 @@ set_date <- function(ds = NULL, tz = "GMT") { ds <- as.character(ds) } - if (min(nchar(ds), na.rm=TRUE) < 12) { - ds <- as.POSIXct(ds, format = "%Y-%m-%d", tz = tz) + # Type should be either character, or an object compatible with lubridate + if (is.character(ds)) { + if (min(nchar(ds), na.rm=TRUE) < 12) { + ds <- as.POSIXct(ds, format = "%Y-%m-%d") + } else { + ds <- as.POSIXct(ds, format = "%Y-%m-%d %H:%M:%S") + } } else { - ds <- as.POSIXct(ds, format = "%Y-%m-%d %H:%M:%S", tz = tz) + if (is.null(attr(ds, "tzone"))) { + # Make sure we don't default to GMT timezone + ds <- lubridate::force_tz(ds) + } + ds <- as.POSIXct(ds) } - attr(ds, "tzone") <- tz return(ds) } @@ -1667,7 +1675,6 @@ make_future_dataframe <- function(m, periods, freq = 'day', dates <- dates[2:(periods + 1)] # Drop the first, which is max(history$ds) if (include_history) { dates <- c(m$history.dates, dates) - attr(dates, "tzone") <- "GMT" } return(data.frame(ds = dates)) } diff --git a/R/tests/testthat/test_prophet.R b/R/tests/testthat/test_prophet.R index 88469dc..b154da3 100644 --- a/R/tests/testthat/test_prophet.R +++ b/R/tests/testthat/test_prophet.R @@ -213,12 +213,12 @@ test_that("fourier_series_weekly", { true.values <- c(0.7818315, 0.6234898, 0.9749279, -0.2225209, 0.4338837, -0.9009689) mat <- prophet:::fourier_series(DATA$ds, 7, 3) - expect_equal(true.values, mat[1, ], tolerance = 1e-6) + expect_equal(true.values, mat[177, ], tolerance = 1e-6) }) test_that("fourier_series_yearly", { - true.values <- c(0.7006152, -0.7135393, -0.9998330, 0.01827656, 0.7262249, - 0.6874572) + true.values <- c(0.70112651, -0.71303690, -0.99985814, 0.01684324, 0.72474500, + 0.68901719) mat <- prophet:::fourier_series(DATA$ds, 365.25, 3) expect_equal(true.values, mat[1, ], tolerance = 1e-6) })