Skip to content

Conversation

@FrancoisPgm
Copy link

Make callback hook functions public and use the skl dunders for init_callback_context.

@github-actions
Copy link

github-actions bot commented Sep 23, 2025

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: e51d632. Link to the linter CI: here

Comment on lines 27 to 34
def on_fit_begin(self, estimator):
"""Callback hook called at the beginning of the estimator's fit method.
Parameters
----------
estimator : estimator instance
Estimator the callback is attached to.
"""
Copy link
Owner

Choose a reason for hiding this comment

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

Let's tweak the sklearn.tests.test_docstrings test to ignore these methods. It can be done in get_all_methods. We can add something like

if isinstance(Klass, Callback) and name in ("on_fit_begin", "on_fit_end", "on_fit_task_end"):
    continue

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I also added the skip in sklearn.tests.test_docstring_parameters.

@jeremiedbb jeremiedbb merged commit 5e75096 into jeremiedbb:base_callbacks_2 Sep 30, 2025
15 of 16 checks passed
@FrancoisPgm FrancoisPgm deleted the renaming_methods branch October 1, 2025 08:36
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.

2 participants