Skip to content
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

register viewer widget with customized key and allow fallback support #430

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions aiidalab_widgets_base/viewers.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,14 @@
AIIDA_VIEWER_MAPPING = {}


def register_viewer_widget(key):
def register_viewer_widget(node_type, key=None):
"""Register widget as a viewer for the given key."""

def registration_decorator(widget):
AIIDA_VIEWER_MAPPING[key] = widget
if key:
AIIDA_VIEWER_MAPPING[(node_type, key)] = widget
else:
AIIDA_VIEWER_MAPPING[node_type] = widget
return widget

return registration_decorator
Expand All @@ -71,7 +74,12 @@ def viewer(obj, downloadable=True, **kwargs):
return obj

try:
_viewer = AIIDA_VIEWER_MAPPING[obj.node_type]
try:
_viewer = AIIDA_VIEWER_MAPPING[(obj.node_type, obj.process_label)]
Copy link
Contributor

Choose a reason for hiding this comment

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

I am wondering, to make this more general we could use the obj.label instead of obj.process_label. I am just not sure if that would be a backwards compatible change for QeApp.

Copy link
Member Author

@unkcpz unkcpz Feb 16, 2023

Choose a reason for hiding this comment

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

Thanks for looking at this. I made this a draft since I did not decided if using process_label is a good design. I was thinking to introduce a rule that stores some standard metadata to extra attributes for use here.
You mentioned in the meeting yesterday that there is a new field that can be used for mapping the process name in TreeNodeWidget, what is that? Is it specific for process or for more general data node?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, it's just a hook for renaming the process_label, see

https://github.com/ispg-group/aiidalab-ispg/blob/7895ed6ce9aa869e1c3a0803064a22a2d2a83fec/workflows/aiidalab_atmospec_workchain/__init__.py#L109

Implemented here:

aiidateam/aiida-core#5713

It would definitely help to make the node tree more user friendly, but is not needed for this particular feature.

except (AttributeError, KeyError):
# Viewer mapping for non-process node and fallback mapping for if process_label
# not specified.
_viewer = AIIDA_VIEWER_MAPPING[obj.node_type]
except (KeyError) as exc:
if obj.node_type in str(exc):
warnings.warn(
Expand Down