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

Fix float(modversion) for x.y.z versions #15

Closed
wants to merge 2 commits into from

Conversation

moto-timo
Copy link

  • Current check_mod_version() fails, for instance, for libparted 1.8.6
  • Rather than indicate that it is a version problem, it complains about the '1.8.6' literal being passed to float()

Current check fails, for instance, for libparted 1.8.6
@moto-timo
Copy link
Author

I don't think the newline character is the problem.

>>> modversion = '1.8.6'
>>> float(modversion)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: invalid literal for float(): 1.8.6
>>> MAJOR = 1
>>> MINOR = 8
>>> MICRO = 6
>>> VERSION = '%d.%d.%d' % (MAJOR, MINOR, MICRO)
>>> VERSION
'1.8.6'
>>> float(VERSION)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: invalid literal for float(): 1.8.6

So my alternate suggestion is to use a regex.

match = re.search('([0-9]+\.[0-9]+)', modversion)
modversion = match.group(0)
if not float(modversion) >= float(version)

Of course, this doesn't check that version can be cast to a float...but the version variable is internally hard coded, so that is probably okay.

I am away from my Fedora machine so I will send another PR later (after it is tested), if this approach sounds acceptable to you.

@dcantrell
Copy link
Owner

I'm not crazy about the regexp method because it's still being too strict on what part of the version number the code is reading, and it's assuming the version number will always be in a specific format. This is moving in the wrong direction.

Better idea, how about using the LooseVersion class from distutils.version. e.g.:

from distutils.version import LooseVersion
if LooseVersion(modversion) <= LooseVersion(version):

The LooseVersion class handles version number formats typically seen in open source projects and does the comparison with alphanumeric values and multiple decimal points.

Thoughts?

@dcantrell
Copy link
Owner

I've changed setup.py to use LooseVersion from distutils.version.

@dcantrell dcantrell closed this Feb 19, 2016
@moto-timo
Copy link
Author

I'll check it out. Thank you.

On Fri, Feb 19, 2016 at 12:26 PM, David Cantrell notifications@github.com
wrote:

I've changed setup.py to use LooseVersion from distutils.version.


Reply to this email directly or view it on GitHub
#15 (comment).

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