-
Notifications
You must be signed in to change notification settings - Fork 305
Use LooseVersion from easybuild.tools in all easyblocks
#3018
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
Conversation
4c685af to
113560b
Compare
easybuild/easyblocks/__init__.py
Outdated
| from distutils.version import LooseVersion | ||
| from pkgutil import extend_path | ||
|
|
||
| from easybuild.tools.loose_version import LooseVersion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's specifically this import that trips up the CI, this is the thing that's used for installed the easyblocks pip package itself.
Really not sure how we fix this though...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well the easy way out is to make the VERSION a string value, rather than a LooseVersion instance, Same goes for VERBOSE_VERSION, so we don't need to import LooseVersion at all here...
Wherever we are using these constants, we have to change accordingly (like in setup.py, test/easyblocks/general.py in here, and in easybuild/tools/version.py in framework).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@appolloford please drop the str()'s here in setup.py:
Line 43 in e6f20bf
| FRAMEWORK_MAJVER = str(VERSION).split('.')[0] |
https://github.com/easybuilders/easybuild-easyblocks/blob/e6f20bf5af27ba0ad650eba70f8719f09a9faa5c/setup.py#L58C1-L58C26
@boegel I'm really not sure what the heck is going on here:
https://github.com/easybuilders/easybuild-easyblocks/blob/develop/test/easyblocks/general.py#L53
LooseVersion from easybuild.tools and sort imports
0c55357 to
13a3e94
Compare
|
So, I suggested to @appolloford to I tried to see in the isort conf if there was some way to limit it to not change so much, like only sorting on type nor alphabetical, but couldn't find anything like that. :( |
6693ec3 to
4d4d246
Compare
4d4d246 to
59f5041
Compare
boegel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
LooseVersion from easybuild.tools and sort importsLooseVersion from easybuild.tools in all easyblocks
|
This is good to go, so I've promoted this from "draft" to "ready to review". A corresponding change in the framework repo may be needed because both |
No description provided.