-
Notifications
You must be signed in to change notification settings - Fork 14
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
Fix issue workchain viewer not designated for workchains except QeAppWorkchain #348
Conversation
b4e4818
to
867b3ad
Compare
a0e260a
to
9a6a8ff
Compare
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.
I have one question, otherwise looks good.
(though I haven't tested it since I don't work with QeApp generally)
Don't worry 😄, I'll ask @superstar54 to give it a test after I revert the wrong change. Thanks a lot for reviewing. |
@superstar54, can you give a look at this? Thanks! |
I tested the example reported in #344. No error anymore. 👍 |
Thanks @superstar54. Can you approve it then? 😄 |
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.
LGTM!
Combined with aiidalab/aiidalab-widgets-base#430 should fix #344fixes #344
The
register_viewer_widget
decorator registers the viewer by class and the new viewer for QeAppWorkchain specifically also become the viewer of other WorkChain. It will raise theKeyError
and raised which is not expected.Instead of raising the exception, nothing is returned. This is a workaround since ideally if there is no new specific viewer defined for a specific work chain, it needs to fall back to the native one of AWB. As tried in aiidalab/aiidalab-widgets-base#430