-
Notifications
You must be signed in to change notification settings - Fork 667
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
Frankenatom fix #4692
Frankenatom fix #4692
Conversation
Hello @MattTDavies! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2024-09-05 09:05:59 UTC |
Linter Bot Results:Hi @MattTDavies! 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 ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #4692 +/- ##
===========================================
- Coverage 93.62% 93.60% -0.03%
===========================================
Files 173 185 +12
Lines 21419 22488 +1069
Branches 3978 3979 +1
===========================================
+ Hits 20053 21049 +996
- Misses 903 976 +73
Partials 463 463 ☔ 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.
lgtm @MattTDavies can you add yourself to the CHANGELOG file authors for 2.8.0?
…o more robustly catch Frankenatoms
67de2d4
to
10b6c76
Compare
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.
LGTM @MattTDavies, could you just check the Developer Certificate of Origin
box and then we can merge.
…MDAnalysis#4692) * Moved index dimension check to initialisation, rather than indexing to more robustly catch Frankenatoms * Added improper atomgroup initialisation test * pep8 fixes * Added CHANGELOG info
Made the "Frankenatom" protection more robust.
Since indexing initialises a new GroupBase, moving the dimension check to initialisation is more robust as it will also capture incorrectly created atomgroups using the
AtomGroup(indices, universe)
initialisation as well as just when indexing a preexisting atomgroup as was covered previously.Fixes #4647
Changes made in this Pull Request:
PR Checklist
Developers certificate of origin
📚 Documentation preview 📚: https://mdanalysis--4692.org.readthedocs.build/en/4692/