search

skops-dev / skops

skops is a Python library helping you share your scikit-learn based models and put them in production
https://skops.readthedocs.io/en/stable/
MIT License
451 stars 54 forks source link

Make skops compatible with scikit-learn 1.6 #443

Open TamaraAtanasoska opened 1 month ago

TamaraAtanasoska commented 1 month ago

Hi! I started to look into the compatibility issues that sckit-learn 1.6 brings.

Specifically the 34 test failures regarding imports from the now refactored sklearn.linear_model._sgd_fastm module (main refactor diff). Example:

ImportError while importing test module '/Users/tamara/FOSS/skops/skops/io/tests/test_visualize.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../miniconda3/envs/skops/lib/python3.10/importlib/__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
skops/io/tests/test_visualize.py:18: in <module>
    import skops.io as sio
skops/io/__init__.py:1: in <module>
    from ._persist import dump, dumps, get_untrusted_types, load, loads
skops/io/_persist.py:22: in <module>
    module = importlib.import_module(module_name, package="skops.io")
../../miniconda3/envs/skops/lib/python3.10/importlib/__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
skops/io/_sklearn.py:15: in <module>
    from sklearn.linear_model._sgd_fast import (
E   ImportError: cannot import name 'Huber' from 'sklearn.linear_model._sgd_fast' (/Users/tamara/miniconda3/envs/skops/lib/python3.10/site-packages/sklearn/linear_model/_sgd_fast.cpython-310-darwin.so)

Talking to a scikit-learn maintainer, it seems the SGD related could be possibly removed, as any of the classes in loss.py don't have a __reduce__ method. Just like this block, the old way can be preserved to support scikit-learn 1.5 or older. The mapping will need to be updated too.

Posting this issue for documentation of the efforts and discussions. I couldn't find a similar one, let me know if I have missed it.

adrinjalali commented 1 month ago

Thanks for checking this @TamaraAtanasoska . Yeah it makes sense what you suggest. We should have those sklearn imports as optional, as in, ignoring import errors if it happens, and the rest of the code should be fine. We should clearly document in the code why those imports are not needed in new sklearn versions.