-
Notifications
You must be signed in to change notification settings - Fork 21
Inject default codes into ResourceSettingsModel
#1368
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
Conversation
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!
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1368 +/- ##
==========================================
- Coverage 72.48% 72.38% -0.11%
==========================================
Files 109 109
Lines 7280 7300 +20
==========================================
+ Hits 5277 5284 +7
- Misses 2003 2016 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1d0af44
to
261df4e
Compare
261df4e
to
20effcc
Compare
I just check and the codes are uploading fine ,but i realize the functional (I went to step two) was selecting the default (PBESol) instead of the one I used (PBE) |
Interesting. I'll have a look in the morning. Update@AndresOrtegaGuerrero Fixed in #1373. I missed setting it in the family-string branch of the if-block. Have a look/test 🙏 |
2ddeda5
to
4a72941
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.
LGreatTM!
Based on #1363
This feature was unintentionally removed in the MVC redesign. Restored here.
Major update
Implementing this PR revealed several other issues, which are also addressed here:
ResourceSettingsPanel.register_code(code_model: CodeModel)
, which is injected into the logic of building the global codes (GlobalResourceSettingsPanel.build_global_codes
, formerlyset_up_codes
.generate_qeapp_workchain
test fixture was usingdeepcopy
on inputs. This results in duplication of AiiDA nodes, which in turn led to a raisedMultipleObjectError
on subsequent use of the fixture due to code duplication of identical labels. See Use shallow copy to avoid creating a new AiiDA node every time #1341 for more details.test_create_builder_default
regression data is corrected here to reflectnum_cpus = 2
, which was not captured beforetraitlets.List(trait=taitlets.Int)
instead oftraitlets.List(trait=taitlets.Int())
, passing unrecognized arguments to constructors, etc.). Fixed here.Peripheral changes