search

business-science / sweep

Extending broom for time series forecasting
https://business-science.github.io/sweep/
155 stars 31 forks source link

Updates on lubridate broke tests #12

Closed vspinu closed 4 years ago

vspinu commented 4 years ago

The recent revdep check broke the class assumption. I dug a bit and it seem that the failure has to do with the upstream timetk::tk_tbl which no longer produces Date but rather POSIXct objects.

I can think only of this change on lubridate side which can be causing issues. Now updating hour,minute,second of a Date object returns a date-time as that's most likely what users expect.

I plan to release in a week or so, but will stop if this is a highly unexpected behavior and it turned to be a lubridate fault at the end. I am afraid I couldn't figure it out by myself by glancing at the code base.

vspinu commented 4 years ago

Identified and fixed upstream in business-science/timetk#32

mdancho84 commented 4 years ago

Hi @vspinu Do you have an example I can check to see what is breaking? I saw the PR but I'd like to see what issue is occuring before I accept.

vspinu commented 4 years ago

Arh, sorry. The checks are opaque failures.

I guess you figured it out as you approved the PR.

Basically all those places which set timezone on Date objects will return POSIXct now. For example:

> FB_tbl    <- FANG %>% filter(symbol == "FB")
> FB_zoo    <- FB_tbl %>% tk_zoo(silent = TRUE)
> class(FB_zoo %>% tk_index(timetk_idx = FALSE))
[1] "POSIXct" "POSIXt"

Therefore, the tests which checked for Date class, failed, both here and in timetk.