♻️ REFACTOR: defaults and widgets of the steps #72
Conversation
Several changes are made to clean up/refactor/modularize the widgets: * Move defaults into `.yaml` that is read and used consistently. Use `importlib_resources` for importing this into modules. * Try to add consistency between the order of these default in the .yaml file and how they are ordered in several commands of ths `SubmitQeAppWorkChainStep`. * Rename `configs` to `settings` (save for `code`, this was included in PR aiidalab#71). * Fix issue where _both_ `OptionsConfig` (now `AdvancedSettings`) and `SubmitQeAppWorkChainStep` would intialize a `PseudoFamilySelector`, leading to issues in correctly assigning the pseudos. Note that there is a small change in UI: The Materials Settings have been moved outside of expert mode, since the layman usually can still understand/decide whether their structure is magnetic/insulating or not. This also seems to have fixed the issue we were having with the UI breaking when disabling expert mode on the "Select Codes" tab.
aiidalab_qe/pseudos.py
Outdated
| installed = traitlets.Bool() | ||
| disable = traitlets.Bool() |
There was a problem hiding this comment.
This should be:
| disable = traitlets.Bool() | |
| disabled = traitlets.Bool() |
And are you actually using that trait?
There was a problem hiding this comment.
No. But didn't you say we needed the disabled trait? 😭
At some point I thought I knew why, so I reinstated it 😎
There was a problem hiding this comment.
No. But didn't you say we needed the
disabledtrait? 😭At some point I thought I knew why, so I reinstated it 😎
The disabled trait should toggle the widget into a "disabled" state when set to True. What that exactly means depends a little bit on the widget so that's why there is no default implementation for this. Ideally, subsequent steps cannot be modified (i.e. are disabled) until the previous step has been confirmed. Hence why most widgets within the app should have this trait and also implement the corresponding behavior.
There was a problem hiding this comment.
Thanks for the explanation :). Makes sense!
|
@csadorf thanks for the review! I think I've covered all your suggestions, but have left some of the comments as unresolved so you can still see my answers to the comments without having to open them. |
|
|
Several changes are made to clean up/refactor/modularize the widgets: * Move defaults into `.yaml` that is read and used consistently. Use `importlib_resources` for importing this into modules. * Try to add consistency between the order of these default in the .yaml file and how they are ordered in several commands of ths `SubmitQeAppWorkChainStep`. * Rename `configs` to `settings` * Fix issue where _both_ `OptionsConfig` (now `AdvancedSettings`) and `SubmitQeAppWorkChainStep` would initialize a `PseudoFamilySelector`, leading to issues in correctly assigning the pseudos. Note that there is a small change in UI: The Materials Settings have been moved outside of expert mode, since the layman usually can still understand/decide whether their structure is magnetic/insulating or not. This also seems to have fixed the issue we were having with the UI breaking when disabling expert mode on the "Select Codes" tab.
Fixes #67
Several changes are made to clean up/refactor/modularize the widgets:
.yamlthat is read and used consistently. Useimportlib_resourcesfor importing this into modules.file and how they are ordered in several commands of ths
SubmitQeAppWorkChainStep.configstosettings(save forcode, this was included inPR 🐛 FIX: do not set params to default for structure change #71).
OptionsConfig(nowAdvancedSettings) andSubmitQeAppWorkChainStepwould intialize aPseudoFamilySelector,leading to issues in correctly assigning the pseudos.
Note that there is a small change in UI: The Materials Settings have
been moved outside of expert mode, since the layman usually can still
understand/decide whether their structure is magnetic/insulating or not.
This also seems to have fixed the issue we were having with the UI
breaking when disabling expert mode on the "Select Codes" tab.