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

[Filing bugs] Error:scikit-learn/sklearn/utils/fixes.py #7898

Closed
willduan opened this issue Nov 17, 2016 · 10 comments · Fixed by #7902
Closed

[Filing bugs] Error:scikit-learn/sklearn/utils/fixes.py #7898

willduan opened this issue Nov 17, 2016 · 10 comments · Fixed by #7902

Comments

@willduan
Copy link
Contributor

willduan commented Nov 17, 2016

There is a new bug in master brunch, scikit-learn/sklearn/utils/fixes.py
in line 406: if np_version < (1, 12, 0): , an error occured, TypeError: unorderable types: str() < int().
It's because in the function :

def _parse_version(version_string):
    version = []
    for x in version_string.split('.'):
        try:
            version.append(int(x))
        except ValueError:
            # x may be of the form dev-1ea1592
            version.append(x)
    return tuple(version)

However, my numpy version is:1.12.0b1, this funciotn return a tuple(1,12,'0b1') , but in line 406, It is compared with integer type, so the error occured.
I just in a simple way to solve it. I change the code to if np_version < (1, 12, '0'): in line 406, just change the third tuple index to string type.
Of course, this is not a common method.

@jnothman
Copy link
Member

I think np_version[:2] < (1, 12) would be more robust. Please submit a
PR, and perhaps identify other cases of comparison in fixes.py where we are
at risk of breaking for non-releases.

On 17 November 2016 at 14:09, willduan notifications@github.com wrote:

There is a new bug in master brunch, scikit-learn/sklearn/utils/fixes.py
in line 406: if np_version < (1, 12, 0): , an error occured, TypeError:
unorderable types: str() < int().
It's because in the function :
def _parse_version(version_string): version = [] for x in
version_string.split('.'): try: version.append(int(x)) except ValueError: #
x may be of the form dev-1ea1592 version.append(x) return tuple(version)
However, my numpy version is:1.12.0b1, this funciotn return a
tuple(1,12,'0b1') , but in line 406, It is compared with integer type, so
the error occured.
I just in a simple way to solve it. I change the code to if np_version <
(1, 12, '0'): in line 406, just change the third tuple index to string
type.
Of course, this is not a common method.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#7898, or mute the
thread
https://github.com/notifications/unsubscribe-auth/AAEz6ylNZpJrZ3d8QXWO_zb_n6NuPZdEks5q-8VYgaJpZM4K0yOg
.

@amueller
Copy link
Member

Can't we use distutils.LooseVersion?

@lesteve
Copy link
Member

lesteve commented Nov 21, 2016

Can't we use distutils.LooseVersion?

Just to prevent duplicating work, the short answer is no and the longer answer is in the PR #7902.

@Cli98
Copy link

Cli98 commented Nov 25, 2016

Clearly slight modifications on parse function could solve the problem. Take the non-number characters out may be enough.
we can also type version number string directly to the fields to past the test.

@Maneetgiri
Copy link

I am not able to open the Fixes.py in my system. I tried opening with 'Edit with IDLE 3.5' but it doesn't open any window. is there anything else that can be done to open and edit that particular file.

@nelson-liu
Copy link
Contributor

@Maneetgiri try opening it with just a regular text editor (e.g. notepad, sublime text, emacs, vim, etc).

@biotechie
Copy link

Thanks, willduan. The fix worked for me.

@jliu118
Copy link

jliu118 commented Mar 15, 2017

@jnothman The ticket is closed but the issue still there:
when you add "x" into list "version"(change to tuple) at return statement if x is NOT digits, this will cause type error later when you compare tuple (1,12,'1rc1) to tuple (1,12,0). Two possible solution.

  1. Use tuple of string.
    Change version.append(int(x))
    to version.append(x)
    Comparison using tuple of strings.
    Change tuple (1,12,0) to ('1', '12', '0') etc.
    A lot of changes.
  2. Simple fix. Extract numeric portion of x if x contains letters using regular expression. Comparison keeps the same.

Hope somebody will merge my fix into next release because I'm NOT a contributor.
...
import re
...
def _parse_version(version_string):
version = []
for x in version_string.split('.'):
try:
version.append(int(x))
except ValueError:
# x may be of the form dev-1ea1592
# version.append(x)
digits=re.match(r'^(\d)+\D',x)
if digits:
version.append(int(digits.group(1)))
else:
# use 0 if no leading digits found
version.append(0)
return tuple(version)

Test environment:
Python 3.4
Numpy 1.12.1rc1
sklearn 0.18.1

jliu118 added a commit to jliu118/scikit-learn that referenced this issue Mar 15, 2017
when you add "x" into list "version"(change to tuple) at return statement if x is NOT digits, this will cause type error later when you compare tuple (1,12,'1rc1) to tuple (1,12,0). Two possible solution.

    Use tuple of string.
    Change version.append(int(x))
    to version.append(x)
    Comparison using tuple of strings.
    Change tuple (1,12,0) to ('1', '12', '0') etc.
    A lot of changes.
    Simple fix. Extract numeric portion of x if x contains letters using regular expression. Comparison keeps the same.

Hope somebody will merge my fix into next release because I'm NOT a contributor.
...
import re
...
def _parse_version(version_string):
version = []
for x in version_string.split('.'):
try:
version.append(int(x))
except ValueError:
# x may be of the form dev-1ea1592
# version.append(x)
digits=re.match(r'^(\d)+\D',x)
if digits:
version.append(int(digits.group(1)))
else:
# use 0 if no leading digits found
version.append(0)
return tuple(version)

Test environment:
Python 3.4
Numpy 1.12.1rc1
sklearn 0.18.1
@lesteve
Copy link
Member

lesteve commented Mar 15, 2017

Look at #7980 which is a more generic issue about version comparison across our codebase. There is already a PR associated to this issue by the way.

@lesteve
Copy link
Member

lesteve commented Mar 15, 2017

Look at #7980 which is a more generic issue about version comparison across our codebase. There is already a PR associated to this issue by the way.

And actually I believe your particular issue is already fixed in master, i.e. the development version.

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 a pull request may close this issue.

9 participants