Skip to content

enhance get_module_path function to auto-detect generic vs software-specific easyblock class names#2502

Merged
vanzod merged 2 commits intoeasybuilders:developfrom
boegel:get_module_path_auto_detect_generic
May 18, 2018
Merged

enhance get_module_path function to auto-detect generic vs software-specific easyblock class names#2502
vanzod merged 2 commits intoeasybuilders:developfrom
boegel:get_module_path_auto_detect_generic

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented May 15, 2018

No description provided.

@boegel boegel added the bug fix label May 15, 2018
@boegel boegel added this to the 3.6.1 milestone May 15, 2018
@boegel
Copy link
Copy Markdown
Member Author

boegel commented May 15, 2018

This is a bug fix for a problem reported by @hajgato, cfr. https://gist.github.com/hajgato/1ff71febf642569f575552661f0fecb4.

Failed to load specified class RPackage for extension ncdf4: No module named rpackage (at easybuild/framework/easyblock.py:1902 in extensions_step)

Using this in the easyconfig file:

exts_classmap = {'ncdf4': 'RPackage'}

The problem was that RPackage was being imported from easybuild.easyblocks.rpackage, while it should be imported from easybuild.easyblocks.generic.rpackage

modulepath = get_module_path(easyblock)
cls = get_class_for(modulepath, class_name)
# we might be dealing with a non-generic easyblock, e.g. with --easyblock is used
# get_module_path figures it out based on specified class name
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The comment sounds a little obscure to me...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, I should probably just drop the comment since get_module_path knows how to handle it? I'll do that.

Copy link
Copy Markdown
Member

@vanzod vanzod left a comment

Choose a reason for hiding this comment

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

LGTM, but I'd rather wait on the review of some other maintainer before merging

Copy link
Copy Markdown
Member

@migueldiascosta migueldiascosta left a comment

Choose a reason for hiding this comment

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

also lgtm

@vanzod vanzod merged commit 5ac4ee3 into easybuilders:develop May 18, 2018
@boegel boegel deleted the get_module_path_auto_detect_generic branch May 18, 2018 17:44
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.

3 participants