Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1231 +/- ##
==========================================
- Coverage 89.58% 89.51% -0.07%
==========================================
Files 70 70
Lines 7860 7851 -9
Branches 956 955 -1
==========================================
- Hits 7041 7028 -13
- Misses 580 584 +4
Partials 239 239 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bendhouseart
force-pushed
the
format-with-ruff
branch
from
eb4325f to
ff05a82
Compare
|
This is fun, moved ruff ignores into code with Ruff also got up to some nonsense with docstring and Assuming the rest of checks go, I'll add in a precommit config and call this done. |
Suppression counts added (by Ruff code): UP031 (79), D102 (48), D103 (33), D101 (31), F601 (22), D417 (20), C408 (16), B006 (13), C403 (13), D100 (11), E741 (11), B904 (11), B028 (10), B007 (9), C414 (9), D104 (7), B905 (7), A002 (6), F841 (6), E501 (6), E402 (4), D404 (3), BLE001 (3), B019 (3), S101 (3), C401 (2), C419 (2), C416 (2), UP030 (2), F811 (2), and EXE002/A001/DTZ011/D301/B004/S110/S307/DTZ005/S701/E711/E721/E731 (1 each). Total # noqa lines added: 389. Total code entries across those lines: 406 Intention is to resolve errors (eventually) in the form of PR's by error code to make reviewing unsafe fixes manageable.
bendhouseart
force-pushed
the
format-with-ruff
branch
from
692535a to
75f8412
Compare
|
Solved some self made issues here and have included links to the relevant files for review with this formatting PR:
Going forward I'll start chipping away at the following errors so we're super consistent and able to get away from |
|
hurrah, the noqa workflow works. |
| @@ -0,0 +1,20 @@ | |||
| ci: | |||
| autoupdate_schedule: monthly | |||
There was a problem hiding this comment.
| autoupdate_schedule: monthly | |
| autoupdate_schedule: quarterly |
| [dependency-groups] | ||
| dev = [ | ||
| "ruff>=0.15.7", | ||
| ] |
There was a problem hiding this comment.
| [dependency-groups] | |
| dev = [ | |
| "ruff>=0.15.7", | |
| ] |
I would remove this and let pre-commit deal with installing ruff otherwise you may have different ruff version (one in pyproject.toml and the other precommit cfg)
| fetch-depth: 0 | ||
| - name: Ensure '# noqa' count does not increase | ||
| run: | | ||
| python tools/check_noqa_budget.py --base-ref "origin/${{ github.base_ref }}" |
There was a problem hiding this comment.
given this is in the tox.ini, could we use tox to run it in CI?
There was a problem hiding this comment.
my hunch is that things in external we should not touch, no?
|
AFAICT ruff is not run in CI, right? |
There was a problem hiding this comment.
I like the approach of making sure the number noqa will go down over time, however I think I would first go with a blanket approach of (the 2 approach are not mutually exclusive):
- ignore the error in this PR for now
- fix all the instances of a given error in a follow up PR
- only leave noqa where errors should be ignored on specific lines
3 participants
Just ran ruff across pybids to make future PR's a bit easier to parse.Dragging this library into the formatting future, PR is large because of it, so I've added the relevant files that should be looked at in this comment below.
I'd really like to enforce a single ruff.toml (or as much can be) for all of our python repo's going forward, so you're going to have a hard time convincing me that what I've done with noqa is necessarily a bad thing.