Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Add temporary fix to _BaseWindowForecaster to handle simultaneous in and out-of-sample forecasts #4812

Merged

Conversation

felipeangelimvieira
Copy link
Contributor

Added temporary fix to _BaseWindowForecaster to handle simultaneous in and out-of-sample forecasts (see #4773). Also added a test to check if NaiveForecaster and BaseWindowForecaster work as expected.

Reference Issues/PRs

Fixes #4773

What does this implement/fix? Explain your changes.

Added an if clause to _BaseWindowForecaster _predict method to correctly concatenate the outputs of in and out of sample forecasts.

Does your contribution introduce a new dependency? If yes, which one?

No

Did you add any tests for the change?

Added test_insample_and_outofsample_forecasting to sktime/forecasting/tests/test_window_forecasters.py

Any other comments?

As this is my first contribution to this package, I'm not sure if I'm skipping some name or other convention. Please feel free to highlight if something should be altered in this PR.

@fkiraly fkiraly added module:forecasting forecasting module: forecasting, incl probabilistic and hierarchical forecasting bugfix Fixes a known bug or removes unintended behavior labels Jul 3, 2023
Copy link
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

Still wondering why the default checks did not catch this.

@fkiraly fkiraly merged commit 3bc463b into sktime:main Jul 3, 2023
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fixes a known bug or removes unintended behavior module:forecasting forecasting module: forecasting, incl probabilistic and hierarchical forecasting
Projects
None yet
2 participants