-
Notifications
You must be signed in to change notification settings - Fork 126
Fix #699: properly deprecated TimeSeriesRegressor.partial_fit method #738
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
base: master
Are you sure you want to change the base?
Fix #699: properly deprecated TimeSeriesRegressor.partial_fit method #738
Conversation
Updated the example to use the new update method instead of the deprecated partial_fit for TimeSeriesRegressor with ENBPI. Adjusted documentation to clarify that update internally calls partial_fit when method='enbpi', and that observed improvements are due to partial_fit's behavior.
Replaces references to `partial_fit` with `update` in the notebook and clarifies the internal behavior of `update`.
Removes extra spaces in the docstring of plot_timeseries_enbpi.py for improved readability and consistency.
Hello @kleeena, thank you for your contribution. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kleeena Actually managed to find some time for a review.
Can you please mention in the existing deprecation warning that the method will be removed in v1.2 ?
``update`` at every step, following [6]. Since ``update`` internally calls the | ||
now-deprecated ``partial_fit`` method when ``method='enbpi'``, | ||
the observed improved coverage and narrower PIs | ||
are due to the underlying behavior of ``partial_fit``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not mention partial_fit at all in this part, now that the method is only internal.
For the legend of the graph below as well.
To be honest, I'm not sure about the correctness of what's being demonstrated here, but that's another problem.
Description
This PR address the issue of TimeSeriesRegressor.partial_method being present by replacing it TimeSeriesRegressor.update in accordance to the
method
attribute, in the following files of the documentation:plot_timeseries_enbpi.py
ts-changepoint.ipynb
Fixes #699
Type of change
Please remove options that are irrelevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist
make lint
make type-check
make tests
make coverage
make doc
make doctest