Skip to content
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] Coordinates #4856

Merged
merged 5 commits into from
Dec 24, 2024
Merged

[fmt] Coordinates #4856

merged 5 commits into from
Dec 24, 2024

Conversation

RMeli
Copy link
Member

@RMeli RMeli commented Dec 22, 2024

@pep8speaks
Copy link

pep8speaks commented Dec 22, 2024

Hello @RMeli! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 145:24: E713 test for membership should be 'not in'
Line 147:80: E501 line too long (118 > 79 characters)
Line 162:80: E501 line too long (82 > 79 characters)
Line 173:80: E501 line too long (108 > 79 characters)
Line 179:80: E501 line too long (99 > 79 characters)
Line 183:80: E501 line too long (83 > 79 characters)
Line 238:80: E501 line too long (80 > 79 characters)
Line 241:80: E501 line too long (211 > 79 characters)

Line 326:80: E501 line too long (108 > 79 characters)
Line 329:80: E501 line too long (156 > 79 characters)

Line 183:80: E501 line too long (88 > 79 characters)
Line 219:80: E501 line too long (88 > 79 characters)
Line 393:21: W503 line break before binary operator
Line 406:21: W503 line break before binary operator
Line 407:21: W503 line break before binary operator
Line 680:13: W503 line break before binary operator
Line 681:13: W503 line break before binary operator

Line 302:12: E713 test for membership should be 'not in'

Line 331:25: W503 line break before binary operator
Line 340:25: W503 line break before binary operator

Line 69:80: E501 line too long (110 > 79 characters)
Line 75:80: E501 line too long (93 > 79 characters)
Line 577:9: W503 line break before binary operator
Line 583:9: W503 line break before binary operator

Line 292:19: E712 comparison to False should be 'if cond is False:' or 'if not cond:'

Line 98:34: E712 comparison to False should be 'if cond is False:' or 'if not cond:'
Line 101:30: E712 comparison to False should be 'if cond is False:' or 'if not cond:'

Line 90:80: E501 line too long (93 > 79 characters)
Line 105:80: E501 line too long (101 > 79 characters)
Line 115:80: E501 line too long (171 > 79 characters)
Line 180:80: E501 line too long (85 > 79 characters)

Line 191:9: W503 line break before binary operator
Line 708:13: W503 line break before binary operator
Line 713:13: W503 line break before binary operator

Line 351:13: W503 line break before binary operator
Line 357:13: W503 line break before binary operator

Line 108:80: E501 line too long (110 > 79 characters)

Line 818:21: W503 line break before binary operator

Line 59:80: E501 line too long (85 > 79 characters)
Line 93:80: E501 line too long (81 > 79 characters)

Line 32:5: W503 line break before binary operator
Line 59:9: W503 line break before binary operator
Line 67:9: W503 line break before binary operator
Line 93:9: W503 line break before binary operator
Line 122:9: W503 line break before binary operator

Line 331:80: E501 line too long (82 > 79 characters)

Line 122:80: E501 line too long (81 > 79 characters)
Line 202:54: E741 ambiguous variable name 'l'

Comment last updated at 2024-12-24 12:56:12 UTC

Copy link

codecov bot commented Dec 22, 2024

Codecov Report

Attention: Patch coverage is 94.03509% with 17 lines in your changes missing coverage. Please review.

Project coverage is 93.63%. Comparing base (29deccc) to head (c141360).
Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
package/MDAnalysis/coordinates/GSD.py 30.00% 7 Missing ⚠️
package/MDAnalysis/coordinates/LAMMPS.py 96.80% 3 Missing ⚠️
package/MDAnalysis/coordinates/CRD.py 92.00% 2 Missing ⚠️
package/MDAnalysis/coordinates/DMS.py 81.81% 2 Missing ⚠️
package/MDAnalysis/coordinates/GRO.py 95.12% 2 Missing ⚠️
package/MDAnalysis/coordinates/PDBQT.py 95.23% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4856      +/-   ##
===========================================
- Coverage    93.65%   93.63%   -0.03%     
===========================================
  Files          177      189      +12     
  Lines        21779    22845    +1066     
  Branches      3064     3064              
===========================================
+ Hits         20398    21391     +993     
- Misses         929     1002      +73     
  Partials       452      452              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@RMeli
Copy link
Member Author

RMeli commented Dec 22, 2024

Fixed trading commas. Good to go IMO.

@RMeli RMeli self-assigned this Dec 22, 2024
@RMeli RMeli requested a review from marinegor December 22, 2024 13:16
Copy link
Member

@orbeckst orbeckst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blanket LGTM — please merge when you're happy with it @RMeli

🎄 🎁

@RMeli RMeli merged commit c08cb79 into MDAnalysis:develop Dec 24, 2024
23 of 24 checks passed
@marinegor
Copy link
Contributor

hey @RMeli sorry it took so long on my side -- LGTM too, no big issues.

Perhaps a little suggestion: could you limit the formatting PRs to ~20-25 files each? Petty reason but my browser keeps crushing during the review process :)

@RMeli RMeli deleted the fmt-coordinates branch January 7, 2025 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants