-
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
use uuid for pseudo #475
use uuid for pseudo #475
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #475 +/- ##
==========================================
+ Coverage 63.73% 64.41% +0.68%
==========================================
Files 40 40
Lines 2702 2844 +142
==========================================
+ Hits 1722 1832 +110
- Misses 980 1012 +32
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
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.
Thanks! The change looks all good, but can you add the implementation to use this for loading the workflow pseudo setting back? The change itself seems not necessary, and I assume when it is used in the actual scenario more change may required, so it makes more sense to look together. You can add commits on top of this.
I added a
The final scenario, which loads all configuration parameters back, can not be done now. It depends on other widgets. It's not good to mix the pseuods with other widgets. |
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.
Thanks! Looks all good to me.
To reload all parameters in the configuration step, one has to save the
configuration_parameters
as an extra attribute of theQeAppWorkChain
process, which means theparameters
can not include an AiiDA Node, so the uuid is used.