-
Notifications
You must be signed in to change notification settings - Fork 30
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
Move coordination number and common data to mctc-lib #135
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #135 +/- ##
==========================================
- Coverage 67.07% 66.75% -0.32%
==========================================
Files 34 33 -1
Lines 4687 4666 -21
Branches 1647 1646 -1
==========================================
- Hits 3144 3115 -29
- Misses 632 636 +4
- Partials 911 915 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Can we reimplement the removed modules based on the new functionality in mctc_ncoord? This would ensure that we don't remove API features in a minor release.
Yes, I can do that. I would use these modules then throughout the code. Still, we should remove it with a later 2.0.0 release (I will open an issue after this is done). |
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.
Looks good to me now.
Let's merge this once we have a new mctc-lib release with the full and final API, such that we don't need to depend on the latest git version.
We can do that. The only remaining update that has to be reviewed before is in tblite, here. In the long term, we (@LSeidler and I) also think about pulling the periodic features (lattice vectors and Wigner-Seitz cell) out of the projects and into mctc-lib. But this shouldn't be a problem, since we can keep the API unchanged. |
PR to move the coordination number to the mctc-lib (see PR #71 there). This avoids duplicated code in all our projects and makes the addition of new coordination numbers simpler. The full functionality and all tests were moved to mctc-lib. Additionally, I moved the pairwise radii to mctc-lib as common data used in future projects at different places.
This PR starts as a draft pointing to the mctc-lib branch until it is merged.