[BUG] Fix Sklearn Models detection by safely importing openml-sklearn

[EmanAbdelhaleem]

Metadata

• Reference Issue: Fixes [BUG] Fix Sklearn Models detection by safely registering SklearnExtension #1542

Details

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

[geetu040]

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

[geetu040]

Keep the "for more information" lines at the very end. Perhaps we can refer to one of the following links for more information

• https://github.com/openml/openml-sklearn
• https://docs.openml.org/python/extensions/

I would prefer the second link, but let me know if you think otherwise.

[EmanAbdelhaleem]

Since we are going to move openml-sklearn back to openml-python, I think the second link would be better.

[fkiraly]

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]

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]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.09%. Comparing base (bd8ae77) to head (f268395).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1556      +/-   ##
==========================================
- Coverage   79.14%   79.09%   -0.05%     
==========================================
  Files          36       36              
  Lines        4320     4325       +5     
==========================================
+ Hits         3419     3421       +2     
- Misses        901      904       +3     

☔ 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.