Skip to content
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

Remove deprecated validate_inputs_base #17

Merged
merged 1 commit into from
Sep 7, 2023

Conversation

mbercx
Copy link
Member

@mbercx mbercx commented May 30, 2023

In aiidateam/aiida-quantumespresso@a389629, the validation of the parent_folder was adapted to no longer rely on splitting up the validation of the top-level inputs in validate_inputs and validate_inputs_base. Instead, a wrapping work chain can just exclude the parent_folder when exposing the inputs of e.g. a PwBaseWorkChain whose parent_folder will be obtained at run-time.

Here we remove the line adapting the validator of the PwCalculation in the nscf namespace of the Wannier90WorkChain to validate_inputs_base, and exclude pw.parent_folder from this namespace instead.

@mbercx mbercx added the pr/blocked Indicate that the PR is blocked until the label is removed. label May 30, 2023
@mbercx mbercx requested a review from qiaojunfeng May 30, 2023 00:04
@mbercx
Copy link
Member Author

mbercx commented May 30, 2023

@qiaojunfeng this PR is blocked until the commit described above is released, but already wanted to give you a heads up.

@qiaojunfeng
Copy link
Collaborator

Thanks, Marnik! The commit message aiidateam/aiida-quantumespresso@a389629 is very good and clear^^

this PR is blocked until the commit described above is released

Do you have any plan of when to release a new version of aiida-qe? Once that it done I can safely merge this

@mbercx
Copy link
Member Author

mbercx commented May 30, 2023

Do you have any plan of when to release a new version of aiida-qe?

Soon, but it's the same old story of "let me just get one more PR in". ^^ In this case it's aiidateam/aiida-quantumespresso#640, but that triggered a small documentation rabbit hole excursion. Still, I'd like to release the new version this week, since also aiidalab-qe is counting on it due to failing tests.

EDIT: Do you have an idea when you'll release v2.X support for these work chains? 🙃

@qiaojunfeng qiaojunfeng changed the base branch from fix/compatibility-aiida-2.0 to develop July 4, 2023 14:15
@mbercx mbercx force-pushed the fix/base-validation branch 2 times, most recently from 8247d1e to 33c0088 Compare August 23, 2023 14:10
@codecov-commenter
Copy link

codecov-commenter commented Aug 23, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.03% ⚠️

Comparison is base (aeceb35) 43.22% compared to head (39ab264) 43.20%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #17      +/-   ##
==========================================
- Coverage   43.22%   43.20%   -0.03%     
==========================================
  Files          44       44              
  Lines        4217     4215       -2     
==========================================
- Hits         1823     1821       -2     
  Misses       2394     2394              
Files Changed Coverage Δ
...c/aiida_wannier90_workflows/workflows/wannier90.py 79.90% <ø> (-0.10%) ⬇️

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

@mbercx
Copy link
Member Author

mbercx commented Aug 23, 2023

@qiaojunfeng I've updated the aiida-quantumespresso dependency to 4.4.0 here, since the change in validation described above is made there.

@mbercx mbercx removed the pr/blocked Indicate that the PR is blocked until the label is removed. label Aug 23, 2023
@mbercx
Copy link
Member Author

mbercx commented Sep 7, 2023

@qiaojunfeng can I go ahead and merge this?

@qiaojunfeng qiaojunfeng merged commit 8350d9a into aiidateam:main Sep 7, 2023
6 checks passed
@qiaojunfeng
Copy link
Collaborator

@qiaojunfeng can I go ahead and merge this?

Sorry for the delay, just merged it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants