config-i1 / smooth

The set of functions used for time series analysis and in forecasting.
89 stars 19 forks source link

Calling c(.) on timeseries objects after loading smooth leads to problems #141

Closed stelsemeyer closed 4 years ago

stelsemeyer commented 4 years ago

I have recognized that the behaviour of calling c on a timeseries changes with a higher version of smooth (compared 2.4.1 vs 2.5.3 for ex.). In the older release c(ts(1)) returns a numeric, whereas in the newer it returns a timeseries.

library("smooth")
#> Registered S3 method overwritten by 'xts':
#>   method     from
#>   as.zoo.xts zoo
#> Registered S3 method overwritten by 'quantmod':
#>   method            from
#>   as.zoo.data.frame zoo
#> Registered S3 methods overwritten by 'forecast':
#>   method             from    
#>   fitted.fracdiff    fracdiff
#>   residuals.fracdiff fracdiff
#> This is package "smooth", v2.4.1
c(ts(1))
#> [1] 1

Created on 2019-09-19 by the reprex package (v0.3.0)

library("smooth")
#> Loading required package: greybox
#> Registered S3 method overwritten by 'xts':
#>   method     from
#>   as.zoo.xts zoo
#> Registered S3 method overwritten by 'quantmod':
#>   method            from
#>   as.zoo.data.frame zoo
#> Registered S3 methods overwritten by 'forecast':
#>   method             from    
#>   fitted.fracdiff    fracdiff
#>   residuals.fracdiff fracdiff
#> Package "greybox", v0.5.4 loaded.
#> This is package "smooth", v2.5.3
c(ts(1))
#> Time Series:
#> Start = 1 
#> End = 1 
#> Frequency = 1 
#> [1] 1

Created on 2019-09-19 by the reprex package (v0.3.0)

This ultimately leads to a recursion when calling tail on a timeseries, since tail.ts calls tail itself (which leads to for ex. many functions of the forecast package like thefaf not working, due to falling for this recursion at some point)

library("smooth")
#> Loading required package: greybox
#> Registered S3 method overwritten by 'xts':
#>   method     from
#>   as.zoo.xts zoo
#> Registered S3 method overwritten by 'quantmod':
#>   method            from
#>   as.zoo.data.frame zoo
#> Registered S3 methods overwritten by 'forecast':
#>   method             from    
#>   fitted.fracdiff    fracdiff
#>   residuals.fracdiff fracdiff
#> Package "greybox", v0.5.4 loaded.
#> This is package "smooth", v2.5.3
try(tail(ts(1)))
#> Error : C stack usage  7971344 is too close to the limit

Created on 2019-09-19 by the reprex package (v0.3.0)

library("smooth")
#> Loading required package: greybox
#> Registered S3 method overwritten by 'xts':
#>   method     from
#>   as.zoo.xts zoo
#> Registered S3 method overwritten by 'quantmod':
#>   method            from
#>   as.zoo.data.frame zoo
#> Registered S3 methods overwritten by 'forecast':
#>   method             from    
#>   fitted.fracdiff    fracdiff
#>   residuals.fracdiff fracdiff
#> Package "greybox", v0.5.4 loaded.
#> This is package "smooth", v2.5.3
try(forecast::thetaf(ts(rep(1:7, 4), frequency = 7)))
#> Error : C stack usage  7969664 is too close to the limit

Created on 2019-09-19 by the reprex package (v0.3.0)

stelsemeyer commented 4 years ago

I guess the problem lies somewhere here: https://github.com/config-i1/greybox/blob/ba2163c4d185c02e1b2ae22d711d599087fed4e9/R/methods.R#L25-L36

config-i1 commented 4 years ago

Ouch. I should have not introduced that method. Thanks for the detailed explanation! This will be removed in the next version of greybox.

config-i1 commented 4 years ago

Done in https://github.com/config-i1/greybox/commit/2ac917869c2be907d1e9e440a1a493caf785bbbc of greybox

stelsemeyer commented 4 years ago

Thank you! 😊👍