Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hooks: scikit-image & scikit-learn: use distribution names for version checks #276

Merged
merged 2 commits into from
Jul 19, 2021

Conversation

rokm
Copy link
Member

@rokm rokm commented Jul 19, 2021

Use distribution names (scikit_image and scikit_learn) for version checks via is_module_satisfies() instead of package names (skimage and sklearn). It is possible to install latest scikit-learn by installing 'sklearn', which installs sklearn==0.0 that ends up messing up the version checks.

Make same adjustment in scikit-image hooks for the sake of consistency (although trying to install skimage shows an error telling user to install scikit-image instead).

@rokm rokm requested review from a team and Legorooj and removed request for a team July 19, 2021 14:49
@rokm
Copy link
Member Author

rokm commented Jul 19, 2021

This is something that I stumbled into while trying to reproduce pyinstaller/pyinstaller#6035 and accidentally installed scikit-learn via pip install sklearn. Due to sklearn==0.0 ending up installed (along with latest version of scikit-learn, the corresponding version checks failed and first caused the test_sklearn to be skipped, and afterwards fail with missing modules in a very similar way as reported in pyinstaller/pyinstaller#6035.

rokm added 2 commits July 19, 2021 16:57
Call is_module_satisfies with distribution name (scikit_learn)
instead of package name (sklearn), because running `pip
install sklearn` actually installs `sklearn==0.0` and pulls
in latest version of `scikit-learn`. So if scikit-learn is
accidentally installed this way, the tests will be skipped
because the version checks based on package name will fail.

Make the same change for scikit-image (scikit_image vs
skimage) for the sake of consistency (although in this
case, installing `skimage` results in an error telling
user to install `scikit-image` instead).
…n checks

Use distribution names (scikit_image and scikit_learn) for
version checks via is_module_satisfies() instead of package
names (skimage and sklearn). It is possible to install latest
scikit-learn by installing 'sklearn', which installs
`sklearn==0.0` that ends up messing up the version checks.

Make same adjustment in scikit-image hooks for the sake of
consistency (although trying to install skimage shows an error
telling user to install scikit-image instead).
@rokm rokm merged commit 50e3f17 into pyinstaller:master Jul 19, 2021
@rokm rokm deleted the use-sklearn-dist-name branch July 19, 2021 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants