Skip to content

Conversation

@EmanAbdelhaleem
Copy link
Contributor

Metadata

Details

Fixed sklearn models detection by safely importing openml-sklearn at openml/runs/__init__.py

Copy link

@geetu040 geetu040 left a comment

Choose a reason for hiding this comment

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

Nice, the safe import logic looks good.

Since this is specific to models and flows I would suggest we move it inside both functions in extensions/functions.py as you suggested before. Though it's still not the best place to put this, but we are restricted by circular imports and can improve the logic afterwards when extension classes are refactored

Also, could you improve the SKLEARN_HINT message, you can suggest to install it using pip install openml-sklearn

Copy link
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

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

Quick question - why does this work, given that openml_sklearn is not actually directly used? Is there some background magic that does something non-explicit, in case the module gets imported?

That feels very dangerous design, if it were so (fully aware that it is not yours).

@EmanAbdelhaleem
Copy link
Contributor Author

EmanAbdelhaleem commented Dec 30, 2025

Is there some background magic that does something non-explicit, in case the module gets imported?

Yes, the SklearnExtension gets registered, the issue was that it wasn't registered, and actually with the current design, it only gets registered when openml-sklearn is imported.
Check here: https://github.com/openml/openml-sklearn/blob/main/src/openml_sklearn/__init__.py#L15C1-L16C1

@codecov-commenter
Copy link

codecov-commenter commented Dec 31, 2025

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.71%. Comparing base (bd8ae77) to head (92091c1).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
openml/extensions/functions.py 60.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1556       +/-   ##
===========================================
- Coverage   79.14%   52.71%   -26.43%     
===========================================
  Files          36       36               
  Lines        4320     4325        +5     
===========================================
- Hits         3419     2280     -1139     
- Misses        901     2045     +1144     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fkiraly fkiraly added the bug label Dec 31, 2025
@fkiraly fkiraly merged commit 8672ffb into openml:main Jan 1, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Fix Sklearn Models detection by safely registering SklearnExtension

4 participants