tidyverts / tsibble

Tidy Temporal Data Frames and Tools
https://tsibble.tidyverts.org
GNU General Public License v3.0
530 stars 49 forks source link

In `split_period`, why not use `lubridate::as.period` instead of `lubridate::seconds_to_period`? #288

Open jerlev opened 1 year ago

jerlev commented 1 year ago

I'm trying to understand what appears to be a limitation of tsibble. The function tsibble:::split_period is used for example by tsibble:::interval_pull.POSIXt, to handle objects of base class difftime:

# Excerpts from tsibble/R/interval.R

split_period <- function(x) {
  output <- seconds_to_period(x)
  list(
    year = output$year, month = output$month, day = output$day,
    hour = output$hour, minute = output$minute, second = output$second
  )
}

#' @export
# Assume date is regularly spaced
interval_pull.POSIXt <- function(x) {
  dttm <- as.double(x)
  nhms <- gcd_interval(dttm)
  period <- split_period(nhms)
  new_interval(
    hour = period$hour,
    minute = period$minute,
    second = period$second %/% 1,
    millisecond = (period$second %% 1 * 1e3) %/% 1,
    microsecond = (period$second %% 1 * 1e3) %% 1 * 1e3
  )
}

In order to transform the base difftime object as a lubridate period object, split_period calls lubridate::seconds_to_period. This assumes that the difftime object will return a number in seconds once cast as a double. However that assumption seems to fail for any difftime object equal to, or longer than one minute. Is that right? If so, why not use lubridate::as.period instead of lubridate::seconds_to_period?