-
Notifications
You must be signed in to change notification settings - Fork 659
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
[fmt] lib #4804
[fmt] lib #4804
Conversation
Hello @RMeli! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2024-11-30 22:14:39 UTC |
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.
Some quick comments.
Thanks @IAlibay for the comments. In this one we indeed have a few oddities that should be discussed. Some are probably OK but I wanted a second opinion, others should stay as they were, and a few ugly things are likely caused by inconsistent use of trailing commas in multi-dimensional arrays (so should be fixable by removing the trailing comma). |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #4804 +/- ##
===========================================
+ Coverage 86.08% 86.41% +0.33%
===========================================
Files 177 189 +12
Lines 21742 22808 +1066
Branches 3055 3055
===========================================
+ Hits 18717 19710 +993
- Misses 2593 2666 +73
Partials 432 432 ☔ 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.
I've added suggestions removing/adding trailing commas when necessary -- it seems that all the "old is better than new" can be resolved with them (plus some extra newlines where necessary), except for one case in tests with the 3x3 matrix formatting with concrete numbers.
Co-authored-by: Egor Marin <marinegor@fastmail.com>
Co-authored-by: Egor Marin <marinegor@fastmail.com>
CI failures are likely un-related to this PR since they showed up also on #4812. |
Format
MDAnalysis/lib
andMDAnalysisTests/lib
.PR Checklist
Developers certificate of origin
📚 Documentation preview 📚: https://mdanalysis--4804.org.readthedocs.build/en/4804/