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] Fix predict_residuals internal data type conversion #4772

Merged
merged 3 commits into from Jul 2, 2023

Conversation

fkiraly
Copy link
Collaborator

@fkiraly fkiraly commented Jun 26, 2023

Fixes #4766

The predict_residuals internal calculation of residuals could break due to different data types for the observed and the predicted, as in some cases the observed would be obtained from self._y which can be data type coerced.

The "optimal" solution would be to not do any coercion or conversion, but for now this should be fixed by converting to the predicted which has the data type consistent with the fit input.

@fkiraly fkiraly added module:forecasting forecasting module: forecasting, incl probabilistic and hierarchical forecasting bugfix Fixes a known bug or removes unintended behavior labels Jun 26, 2023
benHeid
benHeid previously approved these changes Jun 29, 2023
Copy link
Contributor

@benHeid benHeid left a comment

Choose a reason for hiding this comment

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

Thank you for the bugfix. I just added one non-blocking comment.

sktime/forecasting/base/tests/test_base_bugs.py Outdated Show resolved Hide resolved
Copy link
Contributor

@benHeid benHeid left a comment

Choose a reason for hiding this comment

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

Thank you looks good for me.

@benHeid benHeid merged commit 12057c9 into main Jul 2, 2023
24 checks passed
@benHeid benHeid deleted the fix-predict_residuals-convert branch July 2, 2023 13:52
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
Development

Successfully merging this pull request may close these issues.

[BUG] predict_residuals(y=None) does not work if forecaster is fitted with a Series.
2 participants