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

STY: Apply ruff/Pyflakes rules (F) #3681

Merged
merged 6 commits into from
Oct 6, 2024
Merged

Conversation

DimitriPapadopoulos
Copy link
Contributor

Summary

Apply Pyflakes (F) rules, except:

  • F401 (831 errors!)
  • F403
  • F811 (left for later)
  • F821 (fixed some occurrences and left a few for later)
  • F841 (fixed some occurrences and left a few for later)

Copy link

codecov bot commented Sep 22, 2024

Codecov Report

Attention: Patch coverage is 42.85714% with 24 lines in your changes missing coverage. Please review.

Project coverage is 70.84%. Comparing base (83e3903) to head (cb889bf).
Report is 16 commits behind head on master.

Files with missing lines Patch % Lines
nipype/interfaces/cmtk/parcellation.py 0.00% 8 Missing ⚠️
nipype/interfaces/minc/minc.py 0.00% 3 Missing ⚠️
nipype/interfaces/io.py 0.00% 2 Missing ⚠️
nipype/interfaces/mrtrix3/connectivity.py 0.00% 2 Missing ⚠️
nipype/utils/filemanip.py 0.00% 2 Missing ⚠️
nipype/__init__.py 0.00% 1 Missing ⚠️
nipype/interfaces/dipy/simulate.py 0.00% 1 Missing ⚠️
nipype/interfaces/vtkbase.py 0.00% 1 Missing ⚠️
nipype/pipeline/plugins/base.py 0.00% 1 Missing ⚠️
nipype/sphinxext/plot_workflow.py 0.00% 1 Missing ⚠️
... and 2 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3681      +/-   ##
==========================================
- Coverage   70.89%   70.84%   -0.06%     
==========================================
  Files        1277     1277              
  Lines       59212    59116      -96     
  Branches     9799     9801       +2     
==========================================
- Hits        41980    41882      -98     
- Misses      16066    16067       +1     
- Partials     1166     1167       +1     

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

@@ -8,7 +8,6 @@

from nipype.algorithms import misc
from nipype.utils.filemanip import fname_presuffix
from nipype.testing.fixtures import create_analyze_pair_file_in_directory
Copy link
Member

Choose a reason for hiding this comment

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

You've removed a lot of fixtures from the module namespaces. This could be generally rectified by either moving or importing fixtures from nipype.testing.fixtures into nipype.conftest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've put them back for now.

Comment on lines 705 to 706
else:
raise
Copy link
Member

Choose a reason for hiding this comment

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

This was meant to be a for/else, not a try/except/else. What we need to do is instead:

for ...:
    try:
        ...
        break
    except FileNotFoundError as e:
        err = e
        ...
else:
    raise err

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By the way, sleep(2) should be skipped in the last iteration.

F401 imported but unused
F523 `.format` call has unused arguments
F541 f-string without any placeholders
F821 Undefined name

I have left some occurrences, to be fixed in a later pull request.
F841 Local variable is assigned to but never used

I have left a few occurrences, to be examined later.
F901 `raise NotImplemented` should be `raise NotImplementedError`
@effigies effigies merged commit 5083109 into nipy:master Oct 6, 2024
19 checks passed
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