check if code is none in workchain

[superstar54]

fix #506
If no code is not selected, it is None. So, in the workchain, we avoid loading it.

[codecov]

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (fc36106) 78.29% compared to head (6af7550) 78.31%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #505      +/-   ##
==========================================
+ Coverage   78.29%   78.31%   +0.02%     
==========================================
  Files          44       44              
  Lines        3160     3164       +4     
==========================================
+ Hits         2474     2478       +4     
  Misses        686      686              

Flag	Coverage Δ
python-3.10	78.31% <100.00%> (+0.02%)	⬆️
python-3.8	?
python-3.9	78.36% <100.00%> (+0.02%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
src/aiidalab_qe/workflows/__init__.py	57.79% <100.00%> (ø)
tests/test_submit_qe_workchain.py	98.24% <100.00%> (+0.13%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[unkcpz]

Thanks @superstar54, I guess the issue only happens if there are other code, e.g xps involved in the calculation, correct?
The change looks all good for me, can you add a unit test to it?

[superstar54]

I guess the issue only happens if there are other code, e.g xps involved in the calculation, correct?

For all codes. e.g if not calculate pdos, and codes dos is not selected. The dos code is passed as None to the builder.

[superstar54]

The change looks all good for me, can you add a unit test to it?

Thanks for the suggestion. I have added the test.

[unkcpz]

Thanks for adding the test. But I didn’t see what you were testing for. Maybe add an assert statement, what is the value of dos code in the builder?
I also don’t understand why dos code will be None, the drop-down widget only has its value equal to None right after initialization but we already have localhost code for it as default.

But I agree the change is needed for the case such as xps code.

[PNOGillespie]

I also don’t understand why dos code will be None, the drop-down widget only has its value equal to None right after initialization but we already have localhost code for it as default.

If I may add something here (as the person that found this bug originally): I confirm that Xing's fix does work, though I agree that the nature of the issue is strange. From my own testing:

• Running Bands or Xps alone without dos or pdos set returns the error: ValueError: one of the parameters 'identifier', pk', 'uuid' or 'label' has to be specified. You can actually request a Pdos calculation in step 2, set the dos.x and projwfc.x codes in step 3, then un-tick the Pdos calculation in step 2 to avoid the error without running Pdos.
• Removing the line codes = {key: orm.load_node(value) for key, value in codes.items()} and just passing codes without converting the inputs (i.e. codes = parameters["codes"]) works without issues for Bands and Xps, but causes a different crash for Pdos. Unfortunately, I can't find the traceback for that one right now, but it seemed to be that the values for dos and pdos codes were in the form of their UUID, rather than a node like for pw.x.

Hope this helps a little in diagnosing the problem.

[unkcpz]

Thanks a lot for the details @PNOGillespie very helpful.

but it seemed to be that the values for dos and pdos codes were in the form of their UUID, rather than a node like for pw.x.

I have a feeling this could be related to the source of issue. The change look all good and I should not block the PR. approved.

[superstar54]

@PNOGillespie Thanks for the information.