-
Notifications
You must be signed in to change notification settings - Fork 192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added load_plugin_safe method to return calc node if plugin is not available #1185
Changes from 1 commit
382efda
09cf0ff
3d9fb35
2d096b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -125,6 +125,48 @@ def get_plugin(category, name): | |
|
||
return plugin | ||
|
||
def load_plugin_safe(base_class, plugins_module, plugin_type): | ||
""" | ||
It is a copy of load_plugin function to return closely related node class | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would not say it is a copy, I would say it is a 'wrapper' and mention that it does not raise exceptions |
||
if plugin is not available. We are duplicating load_plugin function to not break | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can remove this sentence "We are duplicating load_plugin function to not break its default behaviour." (as above, it's not a copy). |
||
its default behaviour. | ||
|
||
params: Look at the docstring of aiida.common.old_pluginloader.load_plugin for more Info | ||
:return: The plugin class | ||
""" | ||
|
||
try: | ||
PluginClass = load_plugin(base_class, plugins_module, plugin_type) | ||
except MissingPluginError: | ||
nodeParts = plugin_type.partition(".") | ||
baseNodeType = nodeParts[0] | ||
|
||
## data node: temporarily returning base data node. | ||
# In future its better to check the closest available plugin and return it. | ||
# For example if type is "aiida.orm.data.array.kpoints_tmp.KpointsData" | ||
# it should return array data node and not base data node | ||
if baseNodeType == "data": | ||
PluginClass = load_plugin(base_class, plugins_module, 'data.Data') | ||
|
||
## code node | ||
elif baseNodeType == "code": | ||
PluginClass = load_plugin(base_class, plugins_module, 'code.Code') | ||
|
||
## calculation node: for calculation currently we are hardcoding cases | ||
elif baseNodeType == "calculation": | ||
subNodeParts = nodeParts[2].partition(".") | ||
subNodeType = subNodeParts[0] | ||
if subNodeType == "job": | ||
PluginClass = load_plugin(base_class, plugins_module, 'calculation.job.JobCalculation') | ||
elif subNodeType == "inline": | ||
PluginClass = load_plugin(base_class, plugins_module, 'calculation.inline.InlineCalculation') | ||
elif subNodeType == "work": | ||
PluginClass = load_plugin(base_class, plugins_module, 'calculation.work.WorkCalculation') | ||
else: | ||
PluginClass = load_plugin(base_class, plugins_module, 'calculation.Calculation') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. missing else (for baseNodeType). If there is a node of a different baseNodeType, this will crash as PluginClass is not defined. This e.g. will happen for Node (that starts with node.Node. I think).
Maybe add also the case where the baseNodeType is 'node' so you don't get a warning in that case. Can you add a test for Node as well (just to see that it returns Node and does not crash). |
||
|
||
return PluginClass | ||
|
||
|
||
def load_plugin(base_class, plugins_module, plugin_type): | ||
""" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove it from the try/except, as the _safe version does not raise anymore