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] Format topology module and tests #4849

Merged
merged 12 commits into from
Dec 29, 2024
Merged

Conversation

RMeli
Copy link
Member

@RMeli RMeli commented Dec 20, 2024

Format topology module and tests. Some files from active PRs might be excluded (see #4815).


📚 Documentation preview 📚: https://mdanalysis--4849.org.readthedocs.build/en/4849/

@pep8speaks
Copy link

pep8speaks commented Dec 20, 2024

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

Line 129:21: W503 line break before binary operator

Line 63:80: E501 line too long (80 > 79 characters)
Line 64:80: E501 line too long (80 > 79 characters)

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

Line 112:80: E501 line too long (139 > 79 characters)

Line 682:13: W503 line break before binary operator
Line 683:13: W503 line break before binary operator
Line 684:13: W503 line break before binary operator
Line 685:13: W503 line break before binary operator
Line 686:13: W503 line break before binary operator
Line 687:13: W503 line break before binary operator

Line 270:9: E731 do not assign a lambda expression, use a def

Line 164:80: E501 line too long (87 > 79 characters)
Line 343:17: W503 line break before binary operator
Line 343:59: E713 test for membership should be 'not in'
Line 344:17: W503 line break before binary operator
Line 345:17: W503 line break before binary operator

Line 74:13: W503 line break before binary operator
Line 75:13: W503 line break before binary operator
Line 76:13: W503 line break before binary operator

Line 274:9: W503 line break before binary operator
Line 275:9: W503 line break before binary operator
Line 276:9: W503 line break before binary operator
Line 277:9: W503 line break before binary operator
Line 278:9: W503 line break before binary operator
Line 279:9: W503 line break before binary operator
Line 280:9: W503 line break before binary operator
Line 281:9: W503 line break before binary operator
Line 282:9: W503 line break before binary operator

Line 95:13: W503 line break before binary operator
Line 103:13: W503 line break before binary operator
Line 154:21: W503 line break before binary operator
Line 155:21: W503 line break before binary operator
Line 156:21: W503 line break before binary operator
Line 389:80: E501 line too long (81 > 79 characters)
Line 390:80: E501 line too long (81 > 79 characters)
Line 491:70: E231 missing whitespace after ','
Line 793:80: E501 line too long (98 > 79 characters)

Line 148:13: E126 continuation line over-indented for hanging indent
Line 152:9: E121 continuation line under-indented for hanging indent
Line 165:9: W503 line break before binary operator
Line 166:9: W503 line break before binary operator
Line 167:9: W503 line break before binary operator
Line 168:9: W503 line break before binary operator
Line 169:9: W503 line break before binary operator
Line 170:9: W503 line break before binary operator
Line 186:9: W503 line break before binary operator
Line 187:9: W503 line break before binary operator
Line 188:9: W503 line break before binary operator
Line 189:9: W503 line break before binary operator
Line 190:9: W503 line break before binary operator
Line 191:9: W503 line break before binary operator
Line 192:9: W503 line break before binary operator
Line 197:9: W503 line break before binary operator
Line 198:9: W503 line break before binary operator
Line 199:9: W503 line break before binary operator
Line 200:9: W503 line break before binary operator
Line 201:9: W503 line break before binary operator
Line 316:9: E126 continuation line over-indented for hanging indent
Line 319:5: E121 continuation line under-indented for hanging indent

Comment last updated at 2024-12-29 22:02:08 UTC

Copy link

codecov bot commented Dec 20, 2024

Codecov Report

Attention: Patch coverage is 91.87500% with 26 lines in your changes missing coverage. Please review.

Project coverage is 93.63%. Comparing base (b710e57) to head (ca27dec).
Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
package/MDAnalysis/topology/GSDParser.py 16.66% 10 Missing ⚠️
package/MDAnalysis/topology/HoomdXMLParser.py 71.42% 2 Missing and 2 partials ⚠️
package/MDAnalysis/topology/ITPParser.py 93.10% 2 Missing and 2 partials ⚠️
package/MDAnalysis/topology/PSFParser.py 82.35% 3 Missing ⚠️
package/MDAnalysis/topology/CRDParser.py 90.90% 1 Missing ⚠️
package/MDAnalysis/topology/LAMMPSParser.py 97.72% 0 Missing and 1 partial ⚠️
package/MDAnalysis/topology/MOL2Parser.py 94.11% 1 Missing ⚠️
package/MDAnalysis/topology/PQRParser.py 91.66% 0 Missing and 1 partial ⚠️
package/MDAnalysis/topology/guessers.py 94.73% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4849      +/-   ##
===========================================
- Coverage    93.66%   93.63%   -0.03%     
===========================================
  Files          177      189      +12     
  Lines        21787    22853    +1066     
  Branches      3067     3067              
===========================================
+ Hits         20406    21399     +993     
- Misses         929     1002      +73     
  Partials       452      452              

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

Copy link
Member Author

@RMeli RMeli left a comment

Choose a reason for hiding this comment

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

The web UI was really slow with this one. I hope all the review comments were saved.

package/MDAnalysis/topology/tpr/setting.py Outdated Show resolved Hide resolved
package/MDAnalysis/topology/tpr/setting.py Show resolved Hide resolved
testsuite/MDAnalysisTests/topology/test_pdb.py Outdated Show resolved Hide resolved
testsuite/MDAnalysisTests/topology/test_pdb.py Outdated Show resolved Hide resolved
testsuite/MDAnalysisTests/topology/test_top.py Outdated Show resolved Hide resolved
testsuite/MDAnalysisTests/topology/test_tprparser.py Outdated Show resolved Hide resolved
testsuite/MDAnalysisTests/topology/test_tprparser.py Outdated Show resolved Hide resolved
testsuite/MDAnalysisTests/topology/test_tprparser.py Outdated Show resolved Hide resolved
testsuite/MDAnalysisTests/topology/test_top.py Outdated Show resolved Hide resolved
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
Copy link
Member Author

RMeli commented Dec 27, 2024

Wait for #4866 before merging this one.

@RMeli
Copy link
Member Author

RMeli commented Dec 27, 2024

@marinegor this should be good for final review.

@RMeli RMeli removed the request for review from IAlibay December 27, 2024 17:59
@RMeli
Copy link
Member Author

RMeli commented Dec 29, 2024

Wait for #4873 before merging.

@RMeli RMeli merged commit 453be6c into MDAnalysis:develop Dec 29, 2024
22 of 24 checks passed
@RMeli RMeli deleted the fmt-topology branch December 29, 2024 22:54
Copy link
Contributor

@marinegor marinegor left a comment

Choose a reason for hiding this comment

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

retrospective LGTM

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