-
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
make pdos a plugin #459
make pdos a plugin #459
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #459 +/- ##
==========================================
- Coverage 56.69% 56.19% -0.50%
==========================================
Files 36 39 +3
Lines 2593 2657 +64
==========================================
+ Hits 1470 1493 +23
- Misses 1123 1164 +41
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.
Just one minor request, the refactoring looks very nice! Thanks!
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.
I think most of the functions in this file moved from the result.electronic_structure
module, but I didn't see the original codes were deleted.
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.
As we discussed offline, the original code are required by both pdos and bands plugins. It'll be better to find a better way to put the shared modules of multiple plugins. @superstar54 will take care of it after and open a dedicate issue for it. I approve this PR.
Follow PR #456 and PR #458 , this PR makes the
pdos
as a plugin. It implements: