robjhyndman / forecast

Forecasting Functions for Time Series and Linear Models
http://pkg.robjhyndman.com/forecast
1.1k stars 340 forks source link

forecast function is conflicting with "smooth" and "greybox" packages #902

Closed alsabtay closed 1 year ago

alsabtay commented 2 years ago

I get an error when I use forecast() function when I use "forecast" and "smooth" packages at the same time. The forecast() function with auto.arima() model returns two outputs: "pred" and "se".

The problem arises due to call "smooth" package before "forecast" package. I think that the problem arises due to conflict with "greybox::forecast" and "forecast::forecast()" function.

mitchelloharawild commented 2 years ago

Please provide a minimally reproducible example (https://www.tidyverse.org/help/). This is likely due to using greybox::forecast() instead of forecast::forecast() - have you tried using forecast() with forecast::forecast().

On a dev side, the solution would be for both packages to use a mutual package for the generic function, such as the {generics} package. This is underway here: https://github.com/r-lib/generics/issues/63

alsabtay commented 2 years ago

library(smooth) library(forecast) library(Mcomp)

fcs3 = forecast::forecast(forecast::auto.arima(M3[[1899]]$x), h=12, level=95) fcs3$mean

Note: 1) R 4.1.2 installed 2) latest CRAN versions of required packages installed 3) M1 Macbook

mitchelloharawild commented 2 years ago

Oh, this is worse than I had thought - thanks for the MRE.

This is coming from https://github.com/config-i1/greybox/blob/026f5753342b62e0aea6fd44bfb9fdce85801cbe/R/zzz.R#L48-L55

When the greybox package is loaded, it replaces forecast::forecast with greybox::forecast. The {greybox} package also specifies a forecast.default method, which passes the forecast package's Arima model to stats::predict.arima().

@config-i1 - modifying a different package's functions with load hooks doesn't allow users of that package access the function they might want or expect. I think the best solution would be to urge a release of {generics} that includes your PR, and then make changes in {forecast}, {greybox} and {fabletools} to mutually use the generics::forecast generic rather than our own.

config-i1 commented 2 years ago

Yes, this is now switched off from the most recent version of the package, which was submitted on CRAN. I fully support the idea with generics package!

mitchelloharawild commented 2 years ago

Ah, I missed that it was commented out from your .onLoad - thanks for that. In that case, the short term fix would be to update your version of greybox when the next release is accepted on CRAN.

robjhyndman commented 1 year ago

Fixed in https://github.com/robjhyndman/forecast/commit/839062c26afe6d1ca2eb485a65b6f4e265548a88 by importing forecast and accuracy generics.