[Callbacks] FEA Define callback behaviour when cloning an estimator by FrancoisPgm · Pull Request #33340 · scikit-learn/scikit-learn

FrancoisPgm · 2026-02-20T17:24:18Z

Reference Issues/PRs

What does this implement/fix? Explain your changes.

When used on an estimator holding callbacks, clone now discards the callbacks in the clone, raising a warning.
For callback-compatible meta estimator which need to clone sub-estimators, propagate_callbacks now offers a way to clone the sub-estimators while forwarding the callbacks to the clone and silencing the warning.

AI usage disclosure

I used AI assistance for:

Code generation (e.g., when writing an implementation or fixing a bug)
Test/benchmark generation
Documentation (including examples)
Research and understanding

Any other comments?

…ks-aware cloning

jeremiedbb

Thanks for the PR. A few comments.

sklearn/callback/tests/_utils.py

sklearn/callback/_callback_context.py

sklearn/base.py

sklearn/callback/tests/test_callback_support.py

Co-authored-by: Jérémie du Boisberranger <jeremie@probabl.ai>

jeremiedbb

A few more nitpicks. Otherwise LGTM.

Ping @ogrisel for a second review is possible. As discussed in a previous call, this PR makes clone discard the callbacks such that if an estimator with callbacks is passed to a function or meta-estimator that doesn't have callback support, then nothing breaks but the callbacks have no effect.

sklearn/callback/tests/test_callback_context.py

sklearn/callback/_callback_context.py

sklearn/callback/tests/test_callback_support.py

Co-authored-by: Jérémie du Boisberranger <jeremie@probabl.ai>

…e callbacks

jeremiedbb

LGTM. Thanks !

sklearn/callback/tests/test_callback_context.py

Co-authored-by: Jérémie du Boisberranger <jeremie@probabl.ai>

jeremiedbb · 2026-02-26T11:22:59Z

sklearn/callback/_callback_context.py

+        if clone_estimator:
+            from sklearn.base import clone
+
+            with warnings.catch_warnings():
+                warnings.filterwarnings(
+                    "ignore", message="There are callbacks set on the estimator "
+                )
+                cloned_estimator = clone(sub_estimator)
+            if hasattr(sub_estimator, "_skl_callbacks"):
+                cloned_estimator.set_callbacks(sub_estimator._skl_callbacks)
+            sub_estimator = cloned_estimator


nitpick: I'd put the callbacks_to_propagate right before the if not callbacks_to_propagate

jeremiedbb · 2026-02-27T00:08:29Z

sklearn/callback/_callback_context.py

+    def propagate_callback_context(self, sub_estimator, clone_estimator=False):
+        """Propagate the callbacks and the context to a sub-estimator.
+
+        The callbacks are propagated to a clone of the sub-estimator instead if
+        clone_estimator is set to True.


as discussed in the last meeting, let's add an extra arg to clone instead to transfer the callbacks with 3 options: True/False/"warn". "warn" being the default and meaning discard the callbacks with a warning

jeremiedbb · 2026-02-27T15:51:00Z

@ogrisel @StefanieSenger @lesteve I opened #33421 as the alternative that we discussed during last meeting, that is add an option in clone to keep the callbacks or not.

sklearn/callback/_callback_context.py

sklearn/callback/tests/test_callback_context.py

sklearn/callback/_callback_context.py

sklearn/callback/tests/test_callback_context.py

sklearn/callback/tests/test_progressbar.py

sklearn/callback/_callback_context.py

StefanieSenger

Thank you @FrancoisPgm, I have a few comments, but overall looks good to me.

Co-authored-by: Stefanie Senger <91849487+StefanieSenger@users.noreply.github.com>

StefanieSenger

Thanks for addressing my review comments, @FrancoisPgm.

What is lacking for me is a test against what exactly would crash / not work as expected if we do not implement these changes.

I've raised this more concretely here and in the comments before.

ogrisel

Thanks @FrancoisPgm. The PR LGTM besides the following:

sklearn/base.py

ogrisel · 2026-03-10T15:38:50Z

sklearn/base.py

+        warnings.warn(
+            f"There are callbacks set on the estimator {estimator.__class__.__name__} "
+            "being cloned. The callbacks will be discarded in the clone."
+        )  # TODO: add a link to some documentation for callback support.


I think it would be cleaner to create a dedicated warning class under sklearn.exceptions (for instance sklearn.exception.CloneWithCallbacksWarning) to catch instances of this warning explicitly in CallbackContext.clone_and_propagate_callback_context without relying on string message matching.

Very good point, thx :)

sklearn/callback/tests/test_progressbar.py

ogrisel · 2026-03-10T15:50:41Z

sklearn/callback/tests/test_callback_support.py

+    """Test the warning when cloning an estimator with callback registered to it."""
+    estimator = MaxIterEstimator()
+    estimator.set_callbacks(TestingCallback())
+    with pytest.warns(UserWarning, match="There are callbacks set on the estimator "):


This should also be updated to test the specific warning class.

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

lesteve

LGTM, thanks!

As noted by others, it would be nice to figure out exactly what breaks when we don't discard callbacks and add a relevant test.

FrancoisPgm added 4 commits February 20, 2026 17:56

discard callbacks when cloning, use propagate_callbacks to do callbac…

a5cd2dc

…ks-aware cloning

forward the already set callbacks to the clone

5b3fe81

add a test for callback preservation in propagate_callbacks clones

91f534d

also check that the clone is not just the original

37d5dc7

jeremiedbb added Callbacks No Changelog Needed labels Feb 22, 2026

jeremiedbb reviewed Feb 22, 2026

View reviewed changes

sklearn/callback/tests/_utils.py Show resolved Hide resolved

sklearn/callback/_callback_context.py Outdated Show resolved Hide resolved

sklearn/base.py Outdated Show resolved Hide resolved

sklearn/callback/tests/test_callback_support.py Outdated Show resolved Hide resolved

FrancoisPgm and others added 3 commits February 23, 2026 10:59

Update sklearn/base.py

e4ff87b

Co-authored-by: Jérémie du Boisberranger <jeremie@probabl.ai>

review

3962f9e

move test

66cd242

jeremiedbb reviewed Feb 23, 2026

View reviewed changes

sklearn/callback/tests/test_callback_context.py Outdated Show resolved Hide resolved

sklearn/callback/_callback_context.py Outdated Show resolved Hide resolved

sklearn/callback/tests/test_callback_support.py Outdated Show resolved Hide resolved

FrancoisPgm and others added 2 commits February 23, 2026 13:43

Update sklearn/callback/_callback_context.py

aeb3aeb

Co-authored-by: Jérémie du Boisberranger <jeremie@probabl.ai>

more specific warning filter in test and check that clone doesn't hav…

f0014b7

…e callbacks

jeremiedbb approved these changes Feb 23, 2026

View reviewed changes

sklearn/callback/tests/test_callback_context.py Outdated Show resolved Hide resolved

Update sklearn/callback/tests/test_callback_context.py

847c665

Co-authored-by: Jérémie du Boisberranger <jeremie@probabl.ai>

jeremiedbb mentioned this pull request Feb 24, 2026

[Callbacks] TST Add tests to check hook calls #33394

Merged

FrancoisPgm mentioned this pull request Feb 25, 2026

[Callbacks] FEA Add the ScoringMonitor callback #33407

Open

4 tasks

Merge branch 'callbacks' into clone_callbacks

f1721a0

jeremiedbb reviewed Feb 26, 2026

View reviewed changes

jeremiedbb added the Waiting for Second Reviewer First reviewer is done, need a second one! label Feb 26, 2026

jeremiedbb added this to Labs Feb 26, 2026

jeremiedbb moved this to In progress in Labs Feb 26, 2026

jeremiedbb reviewed Feb 27, 2026

View reviewed changes

This was referenced Feb 27, 2026

[Callbacks] Rename propagate_callbacks into propagate_callback_context #33420

Merged

[Callbacks] Alternative to #33340 to cloning with callbacks #33421

Closed

jeremiedbb and others added 4 commits February 27, 2026 16:56

extract renaming

a33352e

move callbacks_to_propagate

9bbc456

fix clonflict

10067f7

Merge branch 'callbacks' into clone_callbacks

d6dc9bb

add clone_and_propagate_callback_context method

b6b1a2f

StefanieSenger reviewed Mar 9, 2026

View reviewed changes

FrancoisPgm and others added 2 commits March 9, 2026 15:00

Update sklearn/callback/_callback_context.py

46282fb

Co-authored-by: Stefanie Senger <91849487+StefanieSenger@users.noreply.github.com>

Update sklearn/callback/_callback_context.py

cbea61a

Co-authored-by: Stefanie Senger <91849487+StefanieSenger@users.noreply.github.com>

StefanieSenger mentioned this pull request Mar 9, 2026

[Callbacks] FEA Add callbacks when cloning an estimator #33490

Closed

4 tasks

FrancoisPgm and others added 4 commits March 9, 2026 17:35

Update sklearn/callback/_callback_context.py

5c8ce62

Co-authored-by: Stefanie Senger <91849487+StefanieSenger@users.noreply.github.com>

Update sklearn/callback/tests/test_callback_context.py

e937471

Co-authored-by: Stefanie Senger <91849487+StefanieSenger@users.noreply.github.com>

Apply suggestions from code review

151d26e

Co-authored-by: Stefanie Senger <91849487+StefanieSenger@users.noreply.github.com>

Merge branch 'callbacks' into clone_callbacks

3417481

github-actions bot added the CI:Linter failure The linter CI is failing on this PR label Mar 9, 2026

linting

5e2469b

github-actions bot removed the CI:Linter failure The linter CI is failing on this PR label Mar 9, 2026

StefanieSenger reviewed Mar 10, 2026

View reviewed changes

make propagate_callback_context return self

daea498

ogrisel approved these changes Mar 10, 2026

View reviewed changes

FrancoisPgm and others added 3 commits March 10, 2026 17:55

Update sklearn/base.py

907ee47

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

Update sklearn/base.py

444db55

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

Update sklearn/callback/tests/test_progressbar.py

5409610

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

github-actions bot added the CI:Linter failure The linter CI is failing on this PR label Mar 10, 2026

FrancoisPgm added 2 commits March 10, 2026 18:46

use ustom warning class

d6f23fb

formatting

ac5d0ff

github-actions bot removed the CI:Linter failure The linter CI is failing on this PR label Mar 10, 2026

lesteve approved these changes Mar 11, 2026

View reviewed changes

lesteve merged commit 3a8ae00 into scikit-learn:callbacks Mar 11, 2026
24 checks passed

github-project-automation bot moved this from In progress to Done in Labs Mar 11, 2026

jeremiedbb mentioned this pull request Mar 11, 2026

[Callbacks] Make clone keep callbacks #33522

Merged

jeremiedbb moved this from Done to In progress in Labs Mar 14, 2026

jeremiedbb removed the status in Labs Mar 14, 2026

jeremiedbb moved this to Done in Labs Mar 14, 2026

Uh oh!

Conversation

FrancoisPgm commented Feb 20, 2026

Reference Issues/PRs

What does this implement/fix? Explain your changes.

AI usage disclosure

Any other comments?

Uh oh!

jeremiedbb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jeremiedbb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jeremiedbb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jeremiedbb Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

jeremiedbb Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

jeremiedbb commented Feb 27, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

StefanieSenger left a comment

Choose a reason for hiding this comment

Uh oh!

StefanieSenger left a comment

Choose a reason for hiding this comment

Uh oh!

ogrisel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ogrisel Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

FrancoisPgm Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ogrisel Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

lesteve left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants