CI Run different sklearn versions in CI #196
Conversation
It does not exist in sklearn 0.24. Since it was chosen more or less arbitrily, it is replaced by GroupKFold.
|
@adrinjalali Some tests are now failing (of course). Any idea why we get:
for sklearn nightly, but only on MacOS? Also, on nightly, some sklearn import apparently triggers:
It doesn't look like we're doing anything wrong here. Perhaps we have to let that specific deprecation slip? |
|
the distutil warning we need to ignore. It's a ton of work on the sklearn side to migrate away from it, but that's not something we can do anything about. The first one we can also ignore, but I'm not sure why it's only on MacOS. @ogrisel @thomasjpfan, anything we should care about here, or in sklearn? |
|
I don't understand either. Both use the same version of scipy 1.9.2 and this problem should have been fixed in scikit-learn dev in scikit-learn/scikit-learn#23633. Maybe the scikit-learn dev wheels are not the same on all the platforms? Since we do not include the git commit hash in the version name, it's hard to tell. |
|
The macos nightly wheels have not been updated for more than 2 months! https://anaconda.org/scipy-wheels-nightly/scikit-learn/files The other wheels have not be updated for a few days because of scikit-learn/scikit-learn#24612 but this is a more recent problem. |
Actually they have, it just that for macos we have older files such as
Let me delete the old macos files. |
|
Done. Feel free to push an empty commit to this PR to trigger the CI again and confirm that this fixed the problem. |
|
Yes, it was fixed, big thanks @ogrisel |
- RANSAC failed to fit in sklearn 0.24 - TfidfTransformer requires sparse data sklearn in 0.24 - Ignore numpy.distutils DeprecationWarning
.github/workflows/build-test.yml
Outdated
| pip uninstall --yes scikit-learn | ||
| if [ ${{ matrix.sklearn_version }} == "nightly" ]; | ||
| then pip install --pre --extra-index https://pypi.anaconda.org/scipy-wheels-nightly/simple scikit-learn; | ||
| else pip install "scikit-learn>=${{ matrix.sklearn_version }}"; |
There was a problem hiding this comment.
shouldn't this be
| else pip install "scikit-learn>=${{ matrix.sklearn_version }}"; | |
| else pip install "scikit-learn=${{ matrix.sklearn_version }}"; |
?
There was a problem hiding this comment.
I set the versions as "0.24,<1.0.dev0" etc. to get the latest patch version. Is there a better way, short of hard-coding it?
There was a problem hiding this comment.
Right, then maybe this is better?
$ pip install scikit-learn~=1.0.0
Collecting scikit-learn~=1.0.0
Downloading scikit_learn-1.0.2-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (26.5 MB)Pulling a VW here: When detecting that the sklearn version is a dev version (like the nightly build), skip the inference tests. This is because the inference environment will fail to build the docker image for those sklearn versions, as they're not on the conda package index. Just installing with pip would not be a solution, because the sklearn nightly build lives on a different index.
|
@adrinjalali I added the skip for the inference tests when using a dev sklearn version |
.github/workflows/build-test.yml
Outdated
| pip uninstall --yes scikit-learn | ||
| if [ ${{ matrix.sklearn_version }} == "nightly" ]; | ||
| then pip install --pre --extra-index https://pypi.anaconda.org/scipy-wheels-nightly/simple scikit-learn; | ||
| else pip install "scikit-learn>=${{ matrix.sklearn_version }}"; |
There was a problem hiding this comment.
Right, then maybe this is better?
$ pip install scikit-learn~=1.0.0
Collecting scikit-learn~=1.0.0
Downloading scikit_learn-1.0.2-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (26.5 MB)
|
@adrinjalali TIL about (here e.g.)
EDIT: Sorry, got it now, you have to indicate the patch version too, it works now. |
adrinjalali
changed the title
Resolves #192