Repository navigation
Conversation
|
also needs fixes in easyblocks that use |
|
Indeed. That might actually be an issue:
I'll check if I can come up with eg. a dict-wrapper that redirects reads and writes to |
|
Ok I added "aliasing" code now. |
|
Once this gets merged I'll update the affected easyblocks |
| _log.nosupport("Obtained value of type '%s' for extra_vars, should be 'dict'" % type(extra_vars), '2.0') | ||
|
|
||
| extra_vars.update({ | ||
| 'load_name': [None, "Name used to load/import this software package, defaults to its name.", CUSTOM], |
There was a problem hiding this comment.
- should use
extension_nameas default value (see Addextension_nameeasyconfig parameter, to specify name of extension to use in generated module file #5259); - should be top-level maybe (also for things that don't derive from
ExtensionEasyBlock)?
There was a problem hiding this comment.
note that for PythonPackage it's not just the extension_name but lower().replace('-', '_')
https://github.com/easybuilders/easybuild-easyblocks/blob/219fb2b77eb9f21efb360406f489896c372f3315/easybuild/easyblocks/generic/pythonpackage.py#L588
There was a problem hiding this comment.
Yes I'd say we keep that logic elsewhere. In Pythonpackage we can overwrite the default:
- Use if set
- If
extension_nameis set use that - Else use
name.lower().replace('-', '_')
But I think the 2nd step (extension_name, then name) makes sense
69accdb to
99c2c30
Compare
|
Ok, I added the extension_name-as-default-load_name commit. That was quite a bit of work to get all cases right. In the end I could just propagate #5260 is still included here |
b5dd00b to
956b63d
Compare
|
On second thought this is not the right way: we should NOT use self.options and try duplicating the Easyconfig parameter into it but rather the other way round: deprecate using it and set the value for options in the Easyconfig for backwards compatibility Adding extension_name showed this: it is not available where we need it and duplicating it into self.options means someone might set it there More general: do we need the Extension class at all? It seems ExtensionEasyblock is all we need together with |
90c3c4b to
45ca829
Compare
|
I added a new commit: Deprecate self.options, options EC parameter and set all in self.cfg That implements the idea I had above: If we have May need adjusting other tests too but wanted to get some feedback first. I can imagine less invasive methods only deprecating access to |
45ca829 to
fc6eae0
Compare
|
@boegel @smoors Fixing the tests is just a small chore. Do you agree with the proposed solution:
This provides us a clear and clean way forward. I guess there was intention to have plain |
fc6eae0 to
287012c
Compare
|
+1 from me for deprecating |
d310ed7 to
743ad84
Compare
|
Sorry, this was pretty complicated to keep backwards compatibility. |
743ad84 to
6d8408a
Compare
0fc9a59 to
4188fe3
Compare
|
We need to get EasyBuild v5.4.1 ASAP (it's way overdue), so I'm moving this one to the next milestone (release after v5.4.1)... |
This allows reuse for different use cases.
…arameter This replaces the misleading `modulename` by `load_name` as it is used to test-load the module/package/software although the naming might differ: - Julia Modules - Python & Perl packages - `import` - `require` - `list | grep` `load` should still be generic enough.
This allows
a) Setting `self.options['modulename']` resulting in `load_name` being set
b) Set `self.options = {'modulename': 'Foo'}` with the same effect
If we have `load_name` and `extension_name` in both `self.cfg` and `self.options` they can have differing values. As options now are always known easyconfig parameters we only need `self.cfg` and can remove `self.options`
This replaces the misleading
modulenamebyload_nameas it is used to test-load the module/package/software although the naming might differ:importrequirelist | greploadshould still be generic enough.I also deprecated passing a dict to
collect_exts_file_infoandget_modulenames: We don't need that and it just complicates things in the function: We always have an extension at that point. I don't see where a dict with anoptionskey could even come from.Fixes #493
Initial draft done by AI, manually verified and adjusted afterwards
Based on
make_deprecated_dict_classfor creating a dict with deprecated/replaced keys #5260nameandversioninEasyConfiginstance ofExtension#5267So those should be merged first