MNT define a parse_version function in sklearn.utils.fixes #17670

hs-nazuna · 2020-06-23T13:57:28Z

Reference Issues/PRs

Resolves #7980
Superses and closes #8301

What does this implement/fix? Explain your changes.

This PR defined a parse_version function in sklearn.utils and replaced some ways to compare versions in some files.

Any other comments?

Fortunately, some files mentioned in #8301 are already fixed by other changes:

Deleted files:
- doc/tutorial/statistical_inference/unsupervised_learning_fixture.py
- sklearn/utils/arpack.py
Other files:
- sklearn/cross_decomposition/pls_.py
- sklearn/feature_extraction/tests/test_image.py
- sklearn/linear_model/least_angle.py
- sklearn/linear_model/omp.py
- sklearn/linear_model/tests/test_logistic.py
- sklearn/metrics/tests/test_classification.py
- sklearn/model_selection/_search.py
- sklearn/preprocessing/label.py
- sklearn/preprocessing/tests/test_data.py
- sklearn/utils/extmath.py
- sklearn/utils/tests/test_extmath.py

…ions

lesteve · 2020-06-23T15:02:42Z

@hs-nazuna thanks for your PR. I edited your PR description to have "Close #8301" so that the previous PR will be closed if we merge this one.

lesteve · 2020-06-23T15:05:19Z

Hmmm I am not too sure why all the CI are red, investigating ...

lesteve · 2020-06-23T15:08:19Z

sklearn/feature_extraction/_hashing_fast.pyx

@@ -89,7 +89,7 @@ def transform(raw_X, Py_ssize_t n_features, dtype,
    indptr_a = np.frombuffer(indptr, dtype=indices_np_dtype)

    if indptr[-1] > np.iinfo(np.int32).max:  # = 2**31 - 1
-        if sp_version < (0, 14):


The minimum Scipy supported version is 0.19.1 this should be removed ideally in another PR.

Can you revert this change?

I'll open an issue about his.

lesteve · 2020-06-23T16:38:32Z

sklearn/_build_utils/__init__.py

 from .pre_build_helpers import basic_check_build
 from .openmp_helpers import check_openmp_support

+from sklearn.utils.fixes import parse_version


I am not too sure, but I suspect you can not import anything from within sklearn here because this file is used to build sklearn ...

As I may completely be wrong, I would advise you to uninstall/reinstall scikit-learn:

pip uninstall scikit-learn -y pip install --no-build-isolation --editable .

and check whether you can reproduce the problem in the CI

I reverted this file to not import scikit-learn in my last commit. Hopefully this should get the CI to at least run the tests (i.e. it will avoid that all the CI break during the build).

…yet ...

sklearn/utils/fixes.py

Co-authored-by: Loïc Estève <loic.esteve@ymail.com>

hs-nazuna · 2020-06-24T07:48:02Z

sklearn/utils/fixes.py:28: error: Incompatible types in assignment (expression has type "Type[LooseVersion]", variable has type "Callable[[str], Tuple[str, ...]]")

How can I avoid this error ...?

The following is my new idea.

def parse_version(version_string):
    try:
        import pkg_resources
        return pkg_resources.parse_version(version_string)
    except ImportError:
        return LooseVersion(version_string)

lesteve · 2020-06-24T07:50:22Z

sklearn/utils/fixes.py

@@ -21,6 +21,12 @@

 from .deprecation import deprecated

+try:
+    import pkg_resources
+    parse_version = pkg_resources.parse_version


Looks like mypy is complaining about the fact that parse_version does not have the same signature in both cases. Maybe @rth knows how to tell mypy to not check this? Otherwise googling is definitely an option.

Build log

sklearn/utils/fixes.py:28: error: Incompatible types in assignment (expression has type "Type[LooseVersion]", variable has type "Callable[[str], Tuple[str, ...]]")

Yeah, you can just add

# type: ignore

at the end of each line. There is no easy fix to make those signatures match. https://mypy.readthedocs.io/en/stable/common_issues.html#spurious-errors-and-locally-silencing-the-checker

@rth Thank you for your advice!
I tried to add it at the end of appropriate lines.

rth

Thanks @hs-nazuna ! A few minor comments but otherwise LGTM.

sklearn/linear_model/tests/test_sgd.py

sklearn/utils/fixes.py

Co-authored-by: Roman Yurchak <rth.yurchak@gmail.com>

lesteve · 2020-06-24T09:07:09Z

I removed _parse_version which was no longer used (and is private so it is fine to remove it without deprecation cycle).

lesteve · 2020-06-24T09:42:51Z

I removed another back-slash (similar to one of @rth's comment). I think this looks good and should be good to merge once the CI is green.

I also ran the tests with the minimal version of dependencies just to check that nothing was badly broken.

rth

Merging, thanks @hs-nazuna !

lesteve · 2020-06-24T12:01:57Z

Thanks a lot @hs-nazuna it feels good to see a 2-3 year old issue getting closed! Thanks @rth as well for proposing the good compromise of using setuptools in the 99% cases where setuptools is installed.

…arn#17670) Co-authored-by: Loïc Estève <loic.esteve@ymail.com> Co-authored-by: Roman Yurchak <rth.yurchak@gmail.com>

Define a parse_version function and replace some ways to compare vers…

a93c9f1

…ions

glemaitre changed the title ~~Define a parse_version function in sklearn.utils~~ MNT define a parse_version function in sklearn.utils.fixes Jun 23, 2020

hs-nazuna added 2 commits June 23, 2020 23:11

modify indents

114732a

modify the implementation

57c093e

lesteve reviewed Jun 23, 2020

View reviewed changes

revert sklearn/feature_extraction/_hashing_fast.pyx

bdefc3c

lesteve reviewed Jun 23, 2020

View reviewed changes

Can not import sklearn in _build_utils because it has not been built …

88f2569

…yet ...

lesteve reviewed Jun 24, 2020

View reviewed changes

sklearn/utils/fixes.py Outdated Show resolved Hide resolved

Update sklearn/utils/fixes.py

60b7004

Co-authored-by: Loïc Estève <loic.esteve@ymail.com>

lesteve reviewed Jun 24, 2020

View reviewed changes

hs-nazuna added 3 commits June 24, 2020 17:12

add type:ignore

c053d2f

resolve conflicts

f46b70f

fix error: at least two spaces before inline comment

2b85753

rth reviewed Jun 24, 2020

View reviewed changes

sklearn/linear_model/tests/test_sgd.py Outdated Show resolved Hide resolved

sklearn/utils/fixes.py Outdated Show resolved Hide resolved

hs-nazuna and others added 4 commits June 24, 2020 17:59

Update sklearn/linear_model/tests/test_sgd.py

bc5bf5b

Co-authored-by: Roman Yurchak <rth.yurchak@gmail.com>

Update sklearn/utils/fixes.py

991e159

Co-authored-by: Roman Yurchak <rth.yurchak@gmail.com>

fix error: visually indented line with same indent as next logical line

4fc0cfa

Remove _parse_version which is no longer used

2421797

Tweak to remove back-slash

e4f8aaf

rth approved these changes Jun 24, 2020

View reviewed changes

rth merged commit c29092d into scikit-learn:master Jun 24, 2020

hs-nazuna deleted the version_comparison branch June 24, 2020 11:39

cmarmo mentioned this pull request Jul 3, 2020

Tests raise Warnings in CI Linux py36_ubuntu_atlas #16685

Closed

hayesall mentioned this pull request Aug 14, 2020

[MNT] 👽 Drop np_version import scikit-learn-contrib/imbalanced-learn#745

Merged

anntzer mentioned this pull request Jan 3, 2021

Avoid importing pkg_resources to speed up import #19098

Closed

rth mentioned this pull request Mar 15, 2021

'<' not supported between instances of 'tuple' and 'Version' error #19672

Closed

Uh oh!

MNT define a parse_version function in sklearn.utils.fixes #17670

MNT define a parse_version function in sklearn.utils.fixes #17670

Conversation

hs-nazuna commented Jun 23, 2020 • edited by lesteve Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

Uh oh!

lesteve commented Jun 23, 2020

Uh oh!

lesteve commented Jun 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lesteve Jun 23, 2020

Choose a reason for hiding this comment

Uh oh!

hs-nazuna Jun 23, 2020

Choose a reason for hiding this comment

Uh oh!

lesteve Jun 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lesteve Jun 24, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hs-nazuna commented Jun 24, 2020

Uh oh!

lesteve Jun 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rth Jun 24, 2020

Choose a reason for hiding this comment

Uh oh!

hs-nazuna Jun 24, 2020

Choose a reason for hiding this comment

Uh oh!

rth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lesteve commented Jun 24, 2020

Uh oh!

lesteve commented Jun 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rth left a comment

Choose a reason for hiding this comment

Uh oh!

lesteve commented Jun 24, 2020

Uh oh!

Uh oh!

hs-nazuna commented Jun 23, 2020 •

edited by lesteve

Loading

lesteve commented Jun 23, 2020 •

edited

Loading

lesteve Jun 23, 2020 •

edited

Loading

lesteve Jun 24, 2020 •

edited

Loading

lesteve commented Jun 24, 2020 •

edited

Loading