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: Adopt ruff for linting and formatting #397

Merged
merged 14 commits into from
Dec 5, 2023

Conversation

effigies
Copy link
Member

@effigies effigies commented Nov 21, 2023

Advantages:

  1. Ruff is extremely fast compared to all Python equivalents.
  2. In addition to formatting, its linter has a --fix flag that can take care of a lot of problems with no user effort.
  3. Consolidates a ridiculous number of rules from a variety of linters/plugins. We can stop using flake8, pyupgrade and isort.
  4. The formatter does sensible things, where it deviates from black
  5. They are considering improving numpy array formatting (Formatter: `np.array` formatting astral-sh/ruff#8452), which is awful in black.
  6. They let us use single quotes again in peace. I'm enforcing them, as I find them significantly less visually cluttering than double quotes.
  7. Works in pre-commit, like everything else.

Disadvantages:

  1. They still don't let hanging comments slide, but that's not a big deal in smriprep.

I've added .git-blame-ignore-rev to hide a lot of this.

Closes #395.

Copy link

codecov bot commented Nov 21, 2023

Codecov Report

Attention: 136 lines in your changes are missing coverage. Please review.

Comparison is base (3b0dcdb) 76.98% compared to head (7e040be) 76.85%.

Files Patch % Lines
smriprep/interfaces/freesurfer.py 23.07% 20 Missing ⚠️
smriprep/cli/run.py 82.27% 14 Missing ⚠️
smriprep/workflows/outputs.py 73.58% 14 Missing ⚠️
smriprep/utils/bids.py 51.85% 13 Missing ⚠️
smriprep/workflows/surfaces.py 73.46% 13 Missing ⚠️
smriprep/workflows/anatomical.py 89.62% 11 Missing ⚠️
smriprep/interfaces/cifti.py 43.75% 9 Missing ⚠️
smriprep/interfaces/reports.py 65.38% 9 Missing ⚠️
smriprep/interfaces/surf.py 76.31% 9 Missing ⚠️
smriprep/interfaces/gifti.py 0.00% 7 Missing ⚠️
... and 7 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #397      +/-   ##
==========================================
- Coverage   76.98%   76.85%   -0.13%     
==========================================
  Files          28       28              
  Lines        2042     2044       +2     
  Branches      240      236       -4     
==========================================
- Hits         1572     1571       -1     
- Misses        438      442       +4     
+ Partials       32       31       -1     
Flag Coverage Δ
ds054 46.03% <46.89%> (+0.05%) ⬆️
pytest 66.43% <61.72%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@effigies effigies merged commit ccc3928 into nipreps:master Dec 5, 2023
15 of 17 checks passed
@effigies effigies deleted the sty/ruff branch December 5, 2023 18:33
effigies added a commit to nipreps/fmriprep that referenced this pull request Feb 5, 2024
Follow-up to nipreps/smriprep#397. This should
replace #3185 and #3147.

I would review this commit-by-commit, and skip
c0f2183, which was a pretty safe run of
`ruff format` and `ruff --fix`. It's dominated by quote changes, so I
would ignore it. Everything else could stand a review.
effigies added a commit that referenced this pull request Mar 20, 2024
0.14.0 (March 11, 2024)

New feature release in the 0.14.x series.

This release restores correct handling of cohort identifiers in templates.
A feature release is warranted due to changes in the workflow structure.

* FIX: Fetch templates during workflow construction (#418)
* FIX: Re-add cohort identifier to template name (#416)
* FIX: Repair FreeSurfer Dependency in Dockerfile (tcsh) (#404)
* FIX: Invert result of skull-strip check in auto mode (#402)
* STY: Adopt ruff for linting and formatting (#397)
* CHORE: Update ruff, ignore certain rules (#419)
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.

Move to ruff?
1 participant