Skip to content

avoid TypeError when checking gompi/iccifort/iimpi/intel versions for deprecation#2792

Merged
akesandgren merged 1 commit intoeasybuilders:4.xfrom
boegel:iimpi_version_check
Mar 2, 2019
Merged

avoid TypeError when checking gompi/iccifort/iimpi/intel versions for deprecation#2792
akesandgren merged 1 commit intoeasybuilders:4.xfrom
boegel:iimpi_version_check

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Mar 2, 2019

Example error that is fixed with this change:

Traceback (most recent call last):
  ...
  File "/Volumes/work/easybuild-framework/easybuild/framework/easyconfig/easyconfig.py", line 390, in __init__
    self.check_deprecated(self.path)
  File "/Volumes/work/easybuild-framework/easybuild/framework/easyconfig/easyconfig.py", line 571, in check_deprecated
    if self.toolchain.is_deprecated():
  File "/Volumes/work/easybuild-framework/easybuild/toolchains/iimpi.py", line 51, in is_deprecated
    elif iimpi_ver > LooseVersion('2000') and iimpi_ver < LooseVersion('2016.01'):
  File "/usr/local/Cellar/python/3.7.2/Frameworks/Python.framework/Versions/3.7/lib/python3.7/distutils/version.py", line 52, in __lt__
    c = self._cmp(other)
  File "/usr/local/Cellar/python/3.7.2/Frameworks/Python.framework/Versions/3.7/lib/python3.7/distutils/version.py", line 337, in _cmp
    if self.version < other.version:
TypeError: '<' not supported between instances of 'str' and 'int'

@boegel boegel added bug fix python3 Python 3 compatibility labels Mar 2, 2019
@boegel boegel added this to the 4.0 milestone Mar 2, 2019
akesandgren
akesandgren approved these changes Mar 2, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@akesandgren akesandgren left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, I don't sese the problem when testing manually with Python 3.6.6

Aha it only happens for instances like
if LooseVersion('2016a') < LooseVersion('2016.1'):
i.e., when that part up to "a" is the same as the first part of '2016.1' since it then have to compare "a" with 1, and distutils LooseVersion is stupid enough to not handle that correctly.
(Not even in Python 2.7, it just accidentally works due to mixed compares)

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Mar 2, 2019

@akesandgren

$ python3
Python 3.7.2 (default, Dec 27 2018, 07:35:06)
[Clang 10.0.0 (clang-1000.11.45.5)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from distutils.version import LooseVersion
>>> LooseVersion('2016a') < LooseVersion('2016.01')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/Cellar/python/3.7.2/Frameworks/Python.framework/Versions/3.7/lib/python3.7/distutils/version.py", line 52, in __lt__
    c = self._cmp(other)
  File "/usr/local/Cellar/python/3.7.2/Frameworks/Python.framework/Versions/3.7/lib/python3.7/distutils/version.py", line 337, in _cmp
    if self.version < other.version:
TypeError: '<' not supported between instances of 'str' and 'int'

See also https://bugs.python.org/issue14894

@akesandgren
Copy link
Copy Markdown
Contributor

akesandgren commented Mar 2, 2019

Going in, thanks @boegel!

@akesandgren akesandgren merged commit fd5f87c into easybuilders:4.x Mar 2, 2019
@boegel boegel deleted the iimpi_version_check branch March 2, 2019 15:54
@boegel boegel mentioned this pull request Mar 3, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@akesandgren akesandgren akesandgren approved these changes

Assignees

No one assigned

Labels

bug fix python3 Python 3 compatibility

Projects

None yet

Milestone

4.0.0

Development

Successfully merging this pull request may close these issues.

2 participants

@boegel @akesandgren