fabletools icon indicating copy to clipboard operation
fabletools copied to clipboard

.model name clash when forecasting from components()

Open gregfoletta opened this issue 4 years ago • 3 comments

Hi,

When forecasting off the back of a components() call, you run into an issue because there is already a '.model' column.

I ran into this when running through Chapter 8, Exercise 10 in FPP3. Of course I could be doing this in the wrong manner, so apologies for this issue if that is the case.

This is similar to #96.

Could it be that functions that generate a 'model':

a. Warn and rename the existing .model columns to something else? Or b. Is this clash detected at the start of the function call and a suitable error thrown?

Here's an example of my code:

library(dplyr)
library(fable)
library(feasts)

tsibbledata::aus_retail %>%
    filter(`Series ID` == 'A3349606J') %>% 
    model(STL(Turnover)) %>% 
    components() %>% 
    model(ETS(season_adjust)) %>% 
    forecast(h = 12)

The errors:

Error: Failed to create output due to bad names.
* Choose another strategy with `names_repair`
Run `rlang::last_error()` to see where the error occurred.

█
├─<error/rlang_error>
│ Failed to create output due to bad names.
│ * Choose another strategy with `names_repair`
└─<error/vctrs_error_names_must_be_unique>
  Names must be unique.
Backtrace:
  1. dplyr::filter(., `Series ID` == "A3349606J")
  1. fabletools::model(., STL(Turnover))
  1. generics::components(.)
  1. fabletools::model(., ETS(season_adjust))
  9. fabletools::forecast(., h = 12)
 12. tidyr:::pivot_longer.data.frame(...)
 13. tidyr::pivot_longer_spec(...)
 19. vctrs::vec_cbind(...)
 21. vctrs:::validate_unique(names = names, arg = arg)
 22. vctrs:::stop_names_must_be_unique(names, arg)
 23. vctrs:::stop_names(...)
 24. vctrs:::stop_vctrs(class = c(class, "vctrs_error_names"), ...)
Run `rlang::last_trace()` to see the full context.

gregfoletta avatar Sep 26 '20 21:09 gregfoletta

OK, so I think in my particular use-case I should have used decomposition_model() - but I think the issue still stands. Perhaps a warning is best?

gregfoletta avatar Sep 26 '20 22:09 gregfoletta

This should be handled as a side-effect of #268, where I agree that a warning should be given in this case.

mitchelloharawild avatar Sep 27 '20 10:09 mitchelloharawild

This approach should also consider other generated key names, such as .rep from generate().

mitchelloharawild avatar Jan 08 '21 11:01 mitchelloharawild