Added atom ID sort to topopology object representation #4190
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello there first time contributor! Welcome to the MDAnalysis community! We ask that all contributors abide by our Code of Conduct and that first time contributors introduce themselves on the developer mailing list so we can get to know you. You can learn more about participating here. Please also add yourself to package/AUTHORS as part of this PR.
Linter Bot Results:Hi @ztimol! Thanks for making this PR. We linted your code and found the following: Some issues were found with the formatting of your code.
Please have a look at the Please note: The |
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## develop #4190 +/- ##
===========================================
- Coverage 93.61% 93.60% -0.01%
===========================================
Files 193 193
Lines 25170 25170
Branches 4059 4059
===========================================
- Hits 23562 23561 -1
Misses 1092 1092
- Partials 516 517 +1
☔ View full report in Codecov by Sentry. |
| @@ -116,7 +116,7 @@ def __hash__(self): | |||
| return hash((self._u, tuple(self.indices))) | |||
|
|
|||
| def __repr__(self): | |||
| indices = (self.indices if self.indices[0] < self.indices[-1] | |||
| indices = np.sort(self.indices if self.indices[0] < self.indices[-1] | |||
There was a problem hiding this comment.
Oops didn't add this to my review:
So looking at the initial issue, I think we want to here instead present the internal stored order of the data, not just sorting by indices. That way you know that your dihedral is in the right i,j,k,l order rather than whatever numeric sort returns.
There was a problem hiding this comment.
ah of course, thanks. Totally misunderstood the initial issue, makes sense now.
Fixes #4181
Entirely possible I missed something but the change seems pretty straightforward. Atom ID topology representations for bonds, angles, dihedrals and other associated objects are now sorted. Happy to update docs/changelogs if necessary.
Changes made in this Pull Request:
PR Checklist
📚 Documentation preview 📚: https://mdanalysis--4190.org.readthedocs.build/en/4190/