convert template constant lists to dicts and export the constants by name #4595
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Those are easier to handle as we can a) Access the names directly by iterating over the dict(-keys) b) Avoid duplications c) Make the semantic clearer d) Allow direct access of values by template name
This allows direct access to the template constants, e.g. for use in hooks. E.g.: `from easybuild.framework.easyconfig.templates import GITHUB_SOURCE`
Flamefire
force-pushed
the
template-constant-dict
branch
from
afba995 to
9281587
Compare
Flamefire
added a commit
to Flamefire/easybuild-easyblocks
that referenced
this pull request
Aug 14, 2024
Required after the conversion in easybuilders/easybuild-framework#4595
1 task
Member
|
@Flamefire A merge conflict emerged here. Also, as a reminder: |
Contributor
Author
Already done -> easybuilders/easybuild-easyblocks#3410 :) |
Flamefire
force-pushed
the
template-constant-dict
branch
from
722253b to
2785c55
Compare
Contributor
|
A few selected test reports uploaded to easybuilders/easybuild-easyblocks#3410 (comment) This seems fine to me. I haven't heard any objects to this. I would like to merge it, just want to check with others first. |
Flamefire
added a commit
to Flamefire/easybuild-easyblocks
that referenced
this pull request
Aug 30, 2024
Required after the conversion in easybuilders/easybuild-framework#4595
boegel
reviewed
Aug 30, 2024
| 'cuda_int_space_sep': 'Space-separated list of integer CUDA compute capabilities', | ||
| 'cuda_int_semicolon_sep': 'Semicolon-separated list of integer CUDA compute capabilities', | ||
| 'cuda_sm_comma_sep': 'Comma-separated list of sm_* values that correspond with CUDA compute capabilities', | ||
| 'cuda_sm_space_sep': 'Space-separated list of sm_* values that correspond with CUDA compute capabilities', |
Member
There was a problem hiding this comment.
software_commit is missing here, I'll open a PR to add it back
bartoldeman
pushed a commit
to ComputeCanada/easybuild-easyblocks
that referenced
this pull request
Apr 3, 2025
Required after the conversion in easybuilders/easybuild-framework#4595
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently access to a constant by name is a bit awkward:
next(e[1] for e in TEMPLATE_CONSTANTS if e[0] == name)Also having them a list of strings or string and tuple of strings isn't obvious that it is rather a mapping of constants to values/documentation and not a real list. Thus it isn't clear either, that each name must only appear once.
This is a non-functional refactoring simplifying the code and improving the expressiveness.
E.g. to get all defined constants we can simply do:
list(TEMPLATE_CONSTANTS)(orlist(TEMPLATE_CONSTANTS.keys()))Additionally I included a change similar to the EasyConfig constants such that you can import them directly, e.g.
from easybuild.framework.easyconfig.templates import GITHUB_SOURCE