Skip to content

[Callbacks] Rename with_fit_callbacks -> with_callbacks#33547

Merged
jeremiedbb merged 3 commits intoscikit-learn:callbacksfrom
jeremiedbb:more-generic-decorator
Mar 16, 2026
Merged

[Callbacks] Rename with_fit_callbacks -> with_callbacks#33547
jeremiedbb merged 3 commits intoscikit-learn:callbacksfrom
jeremiedbb:more-generic-decorator

Conversation

@jeremiedbb
Copy link
Copy Markdown
Member

@jeremiedbb jeremiedbb commented Mar 15, 2026

As reported in scikit-learn/enhancement_proposals#90 (comment) in the SLEP, the current name, with_fit_callbacks, is too specific since we'll likely want to reuse this decorator for other methods when we extend the callback API.

So let's directly go with a generic with_callbacks.
Since, as a decorator, it receives the decorated method as first arg, it will be easy to use it for methods other than fit in the future, specializing the behavior for the different methods if needed.

(note that the callback_management_context context manager used inside the decorator is currently dedicated to fit, but that's fine because this one is private, so we'll be able to modify it later as needed without touching the public api)

ping @ogrisel @FrancoisPgm @StefanieSenger @lesteve

@jeremiedbb jeremiedbb added No Changelog Needed Quick Review For PRs that are quick to review Callbacks labels Mar 15, 2026
Copy link
Copy Markdown
Member

@lesteve lesteve left a comment

Choose a reason for hiding this comment

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

Sounds fine, with #33545 merged, I guess the API doc needs to be changed to have with_callbacks instead of with_fit_callbacks.

Copy link
Copy Markdown
Member

@StefanieSenger StefanieSenger left a comment

Choose a reason for hiding this comment

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

Yes, thank you @jeremiedbb! (Can be merged after the conflict is resolved.)

@jeremiedbb jeremiedbb merged commit 8db1f33 into scikit-learn:callbacks Mar 16, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Callbacks No Changelog Needed Quick Review For PRs that are quick to review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants