Skip to content

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kleeena
Copy link

@kleeena kleeena commented Aug 7, 2025

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.

  • Bug fix (non-breaking change which fixes an issue)
  • This change requires a documentation update

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

  • Ran the make doc, make doctest commands. Both were successful
  • For .ipynb, double-checked using colab.

Checklist

  • I have read the contributing guidelines
  • I have updated the HISTORY.rst and AUTHORS.rst files
  • Linting passes successfully: make lint
  • Typing passes successfully: make type-check
  • Unit tests pass successfully: make tests
  • Coverage is 100%: make coverage
  • When updating documentation: doc builds successfully and without warnings: make doc
  • When updating documentation: code examples in doc run successfully: make doctest

kleeena added 4 commits August 7, 2025 16:18
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.
@Valentin-Laurent
Copy link
Collaborator

Hello @kleeena, thank you for your contribution.
The core devs are off for a couple weeks, please expect a delay in the review.
Cheers,

Copilot

This comment was marked as resolved.

@scikit-learn-contrib scikit-learn-contrib deleted a comment from Copilot AI Aug 10, 2025
Copy link
Collaborator

@Valentin-Laurent Valentin-Laurent left a 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``.
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Properly deprecate TimeSeriesRegressor.partial_fit method
2 participants