Skip to content

MNT: Enforce ruff/Pyflakes rules (F)#27309

Merged
jorenham merged 7 commits into
numpy:mainfrom
DimitriPapadopoulos:F
May 12, 2025
Merged

MNT: Enforce ruff/Pyflakes rules (F)#27309
jorenham merged 7 commits into
numpy:mainfrom
DimitriPapadopoulos:F

Conversation

@DimitriPapadopoulos

@DimitriPapadopoulos DimitriPapadopoulos commented Aug 29, 2024

Copy link
Copy Markdown
Contributor

In preparation for #24994.

The purpose of this PR is to show the kind of changes involved by F rules, because ruff enables F rules by default.

The idea is not necessarily to apply these rules. Rather I would like to start a discussion whether it we should apply or disable these F rules, if pycodestyle was replaced by ruff.

Now that we have switched to ruff, try to enable Pyflakes F rules.

I have left out F821. Relevant changes involve circular imports and extensive changes. This is best left to a future PR.

@DimitriPapadopoulos DimitriPapadopoulos changed the title MAINT: Start applying ruff/Pyflakes rules (F) MNT: Enforce ruff/Pyflakes rules (F) Apr 18, 2025
Comment thread benchmarks/benchmarks/bench_array_coercion.py
Comment thread numpy/__init__.py Outdated
Comment thread numpy/__init__.pyi
Comment thread numpy/__init__.pyi
Comment thread numpy/_core/_add_newdocs.py
Comment thread numpy/_core/tests/_natype.py Outdated
Comment thread numpy/_core/tests/_natype.py Outdated
Comment thread numpy/_core/tests/_natype.py Outdated
Comment thread numpy/_core/tests/test_overrides.py
Comment thread numpy/_core/tests/test_ufunc.py Outdated
@DimitriPapadopoulos
DimitriPapadopoulos marked this pull request as ready for review April 18, 2025 14:20
@DimitriPapadopoulos

DimitriPapadopoulos commented Apr 18, 2025

Copy link
Copy Markdown
Contributor Author

I'm getting circular imports trying to address ruff suggestions.

@jorenham jorenham left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I noticed that there are a lot of F401 (unused-import) ignores. Maybe we should just disable it structurally, e.g. for all __init__.py in the lint.extend-per-file-ignores?

Comment thread benchmarks/benchmarks/bench_array_coercion.py
Comment thread numpy/_core/_add_newdocs.py
Comment thread numpy/_core/arrayprint.py Outdated
Comment thread numpy/_core/multiarray.py Outdated
Comment thread numpy/_core/tests/test_ufunc.py
Comment thread numpy/ma/extras.py Outdated
Comment thread ruff.toml Outdated
Comment thread ruff.toml Outdated
Comment thread ruff.toml Outdated
Comment thread ruff.toml
@jorenham

jorenham commented Apr 19, 2025

Copy link
Copy Markdown
Member

In numtype I've been using nested (hierarchical) .ruff.toml quite a bit. It avoids the main ruff config from becoming too monolithic when there are many per-file-ignores. For example in the src/numpy-stubs/@test dir there's a .ruff.toml with the contents:

extend = "../../../pyproject.toml"
line-length = 88

[lint]
extend-ignore = [
    "D",       # pydocstyle
    "ERA",     # eradicate
    "S",       # flake8-bandit
    "T",       # flake8-print
    "PLR2004", # pylint: magic-value-comparisons
]

Perhaps something like that could also help for numpy?

@DimitriPapadopoulos

Copy link
Copy Markdown
Contributor Author

The amount of per file ignores remains limited for now. I suggest we revisit hierarchical ruff.toml files when adding more rules in the future.

@github-actions

This comment has been minimized.

@DimitriPapadopoulos

Copy link
Copy Markdown
Contributor Author

I'm not sure how to use mypy_primer, there weren't too many typing-related changes here. I have rebased just in case.

@github-actions

This comment has been minimized.

@jorenham

jorenham commented Apr 21, 2025

Copy link
Copy Markdown
Member

Diff from mypy_primer, showing the effect of this PR on type check results on a corpus of open source code:

vision (https://github.com/pytorch/vision)
- torchvision/utils.py:271: error: Unused "type: ignore" comment  [unused-ignore]

https://github.com/pytorch/vision/blob/main/torchvision/utils.py#L271

🤷🏻

@DimitriPapadopoulos

Copy link
Copy Markdown
Contributor Author

Rebased to fix conflicts.

@jorenham

jorenham commented May 1, 2025

Copy link
Copy Markdown
Member

needs another rebase

@DimitriPapadopoulos

Copy link
Copy Markdown
Contributor Author

Rebased to fix conflicts.

@DimitriPapadopoulos

Copy link
Copy Markdown
Contributor Author

Rebased.

@jorenham

Copy link
Copy Markdown
Member

Rebased.

it needs another, I'm afraid

@DimitriPapadopoulos

Copy link
Copy Markdown
Contributor Author

Rebased.

@jorenham
jorenham merged commit 95c2e05 into numpy:main May 12, 2025
@jorenham

Copy link
Copy Markdown
Member

Thanks Dimitri

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants