Skip to content

[Callbacks] FIX Make set_callbacks accept only instances#33574

Merged
jeremiedbb merged 5 commits intoscikit-learn:callbacksfrom
FrancoisPgm:set_callbacks_check
Mar 19, 2026
Merged

[Callbacks] FIX Make set_callbacks accept only instances#33574
jeremiedbb merged 5 commits intoscikit-learn:callbacksfrom
FrancoisPgm:set_callbacks_check

Conversation

@FrancoisPgm
Copy link
Copy Markdown
Contributor

Reference Issues/PRs

Towards #27676

What does this implement/fix? Explain your changes.

CallbackSupportMixin.set_callbacks verify that the provided callbacks are valid by using isinstance with the callback protocol. But isinstance with protocols also accepts classes, so passing the class of a valid callback would pass the test in set_callbacks and create problems later.

The change simply adds a inspect.isclass test to reject classes and only accept instances.

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?

ping @jeremiedbb @ogrisel @StefanieSenger @lesteve

Co-authored-by: Jérémie du Boisberranger <jeremie@probabl.ai>
@github-actions github-actions bot added the CI:Linter failure The linter CI is failing on this PR label Mar 18, 2026
@github-actions github-actions bot removed the CI:Linter failure The linter CI is failing on this PR label Mar 18, 2026
FrancoisPgm and others added 2 commits March 18, 2026 17:42
@jeremiedbb jeremiedbb merged commit 3a283ae into scikit-learn:callbacks Mar 19, 2026
25 checks passed
@FrancoisPgm FrancoisPgm deleted the set_callbacks_check branch March 30, 2026 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants