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

fix: pyflakes style #1953

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
Draft

fix: pyflakes style #1953

wants to merge 10 commits into from

Conversation

jorgepiloto
Copy link
Member

@jorgepiloto jorgepiloto commented Dec 4, 2024

This is related with #296. It enables the PyFlakes (F) rule. Most of the changes include:

  • Remove unused variables
  • Remove duplicated tests
  • Usage of __all__ in __init__ files

@jorgepiloto jorgepiloto marked this pull request as draft December 4, 2024 09:22
Copy link

codecov bot commented Dec 4, 2024

Codecov Report

Attention: Patch coverage is 60.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 87.35%. Comparing base (b7f89f7) to head (6f89fce).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1953      +/-   ##
==========================================
- Coverage   88.41%   87.35%   -1.07%     
==========================================
  Files          89       89              
  Lines       10258    10248      -10     
==========================================
- Hits         9070     8952     -118     
- Misses       1188     1296     +108     

@jorgepiloto
Copy link
Member Author

Checking the decrease in coverage. Some tests were duplicated and exactly the same.

@jorgepiloto jorgepiloto force-pushed the fix/pyflakes branch 8 times, most recently from a9cd044 to cf892ad Compare December 4, 2024 14:30
Copy link
Member Author

Choose a reason for hiding this comment

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

This test slightly differs from another named exactly the same within this file. To be reviewed...

@jorgepiloto
Copy link
Member Author

Progress on this for the next days:

  • Restore unused properties
  • Verify their value in Pytest

This solves the issues for coverage and codacy.

Example:

mesh = Object.mesh
assert np.assert_allclose(mesh, mesh_expected, atol, rtol)

@moe-ad
Copy link
Contributor

moe-ad commented Jan 3, 2025

I do not see how we can boost coverage on the erring lines. Most are modified f-strings or exception statements. If the lines are really being covered before now, there is no reason why they shouldn't be covered after the changes, at least I can't think of any.

@moe-ad moe-ad removed their assignment Jan 3, 2025
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.

2 participants