Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Flip order of `is_prerelease` and version check for clarity.

Co-authored-by: Kenneth Hoste <kenneth.hoste@ugent.be>
  • Loading branch information
bartoldeman and boegel authored Mar 11, 2024
1 parent 8f22651 commit a6fda46
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 8 deletions.
6 changes: 3 additions & 3 deletions easybuild/tools/modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,22 +254,22 @@ def set_and_check_version(self):
version = LooseVersion(self.version)
if self.REQ_VERSION is not None:
self.log.debug("Required minimum %s version defined: %s", self.NAME, self.REQ_VERSION)
if version.is_prerelease(self.REQ_VERSION, ['rc', '-beta']) or version < self.REQ_VERSION:
if version < self.REQ_VERSION or version.is_prerelease(self.REQ_VERSION, ['rc', '-beta']):
raise EasyBuildError("EasyBuild requires %s >= v%s, found v%s",
self.NAME, self.REQ_VERSION, self.version)
else:
self.log.debug('%s version %s matches requirement >= %s', self.NAME, self.version, self.REQ_VERSION)

if self.DEPR_VERSION is not None:
self.log.debug("Deprecated %s version limit defined: %s", self.NAME, self.DEPR_VERSION)
if version.is_prerelease(self.DEPR_VERSION, ['rc', '-beta']) or version < self.DEPR_VERSION:
if version < self.DEPR_VERSION or version.is_prerelease(self.DEPR_VERSION, ['rc', '-beta']):
depr_msg = "Support for %s version < %s is deprecated, " % (self.NAME, self.DEPR_VERSION)
depr_msg += "found version %s" % self.version
self.log.deprecated(depr_msg, '6.0')

if self.MAX_VERSION is not None:
self.log.debug("Maximum allowed %s version defined: %s", self.NAME, self.MAX_VERSION)
if not version.is_prerelease(self.MAX_VERSION, ['rc', '-beta']) and self.version > self.MAX_VERSION:
if self.version > self.MAX_VERSION and not version.is_prerelease(self.MAX_VERSION, ['rc', '-beta']):
raise EasyBuildError("EasyBuild requires %s <= v%s, found v%s",
self.NAME, self.MAX_VERSION, self.version)
else:
Expand Down
10 changes: 5 additions & 5 deletions test/framework/modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,14 +228,14 @@ def test_avail(self):
ms = self.modtool.available()
version = LooseVersion(self.modtool.version)

if isinstance(self.modtool, Lmod) and not version.is_prerelease('5.7.5', ['rc']) and version >= '5.7.5':
if isinstance(self.modtool, Lmod) and version >= '5.7.5' and not version.is_prerelease('5.7.5', ['rc']):
# with recent versions of Lmod, also the hidden modules are included in the output of 'avail'
self.assertEqual(len(ms), TEST_MODULES_COUNT + 3)
self.assertIn('bzip2/.1.0.6', ms)
self.assertIn('toy/.0.0-deps', ms)
self.assertIn('OpenMPI/.2.1.2-GCC-6.4.0-2.28', ms)
elif (isinstance(self.modtool, EnvironmentModules)
and not version.is_prerelease('4.6.0', ['-beta'])) and version >= '4.6.0':
and version >= '4.6.0' and not version.is_prerelease('4.6.0', ['-beta'])):
# bzip2/.1.0.6 is not there, since that's a module file in Lua syntax
self.assertEqual(len(ms), TEST_MODULES_COUNT + 2)
self.assertIn('toy/.0.0-deps', ms)
Expand Down Expand Up @@ -316,7 +316,7 @@ def test_exist(self):
avail_mods = self.modtool.available()
self.assertIn('Java/1.8.0_181', avail_mods)
version = LooseVersion(self.modtool.version)
if isinstance(self.modtool, Lmod) and not version.is_prerelease('7.0', ['rc']) and version >= '7.0':
if isinstance(self.modtool, Lmod) and version >= '7.0' and not version.is_prerelease('7.0', ['rc']):
self.assertIn('Java/1.8', avail_mods)
self.assertIn('Java/site_default', avail_mods)
self.assertIn('JavaAlias', avail_mods)
Expand Down Expand Up @@ -376,7 +376,7 @@ def test_exist(self):
self.assertEqual(self.modtool.exist(['Core/Java/1.8', 'Core/Java/site_default']), [True, True])

# also check with .modulerc.lua for Lmod 7.8 or newer
if isinstance(self.modtool, Lmod) and not version.is_prerelease('7.8', ['rc']) and version >= '7.8':
if isinstance(self.modtool, Lmod) and version >= '7.8' and not version.is_prerelease('7.8', ['rc']):
shutil.move(os.path.join(self.test_prefix, 'Core', 'Java'), java_mod_dir)
reset_module_caches()

Expand Down Expand Up @@ -408,7 +408,7 @@ def test_exist(self):
self.assertEqual(self.modtool.exist(['Core/Java/site_default']), [True])

# Test alias in home directory .modulerc
if isinstance(self.modtool, Lmod) and not version.is_prerelease('7.0', ['rc']) or version >= '7.0':
if isinstance(self.modtool, Lmod) and version >= '7.0' and not version.is_prerelease('7.0', ['rc']):
# Required or temporary HOME would be in MODULEPATH already
self.init_testmods()
# Sanity check: Module aliases don't exist yet
Expand Down

0 comments on commit a6fda46

Please sign in to comment.