Skip to content

use enumerate where applicable + fix for ModuleGenerator._generate_multi_deps_list#4720

Merged
boegel merged 2 commits intoeasybuilders:5.0.xfrom
Flamefire:range-len-to-enumerate
Jan 29, 2025
Merged

use enumerate where applicable + fix for ModuleGenerator._generate_multi_deps_list#4720
boegel merged 2 commits intoeasybuilders:5.0.xfrom
Flamefire:range-len-to-enumerate

Conversation

@Flamefire
Copy link
Copy Markdown
Contributor

PyLint found uses of range(len(...)) where the index was then used to access an element.

Using enumerate yields the value together with the index making the intention clearer (and the code likely faster).

This also led to a nice simplification possibility in make_module_dep, get_parsed_multi_deps and configobj.walk and a possible bugfix in _generate_multi_deps_list:

The code generating the load statement uses the first module of each name in self.app.cfg.multi_deps but the code generating the help text has an additional version check. We don't need that and it can possibly be wrong if the order of the versions was different somehow.
I simply collect all modules with the given name using a (fast) list comprehension and mark the first as the default

The multi-dep module loaded by default is the first entry in
`self-cfg-multi_deps` for each name.
Using the version order from the easyconfig could lead to
inconsistencies between description and behavior.
In some places we require the index and value of a list and using
`range(len(...))` is not optimal for that.
Using `enumerate` yields the value together with the index making the
intention clearer (and the code likely faster).
@boegel boegel added this to the release after 4.9.4 milestone Dec 18, 2024
Copy link
Copy Markdown
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

lgtm

@boegel boegel changed the title use enumerate where applicable fixing _generate_multi_deps_list use enumerate where applicable + fix for ModuleGenerator._generate_multi_deps_list Jan 29, 2025
@boegel boegel added the EasyBuild-5.0 EasyBuild 5.0 label Jan 29, 2025
@boegel boegel modified the milestones: release after 4.9.4, 5.0 Jan 29, 2025
@boegel boegel merged commit 3272aa0 into easybuilders:5.0.x Jan 29, 2025
@Flamefire Flamefire deleted the range-len-to-enumerate branch February 2, 2025 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants