Skip to content

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

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

DimitriPapadopoulos
Copy link
Contributor

@DimitriPapadopoulos DimitriPapadopoulos commented Aug 29, 2024

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
@DimitriPapadopoulos DimitriPapadopoulos marked this pull request as ready for review April 18, 2025 14:20
@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Apr 18, 2025

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

Copy link
Member

@jorenham jorenham left a comment

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?

@jorenham
Copy link
Member

jorenham commented Apr 19, 2025

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
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.

This comment has been minimized.

@DimitriPapadopoulos
Copy link
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.

This comment has been minimized.

@jorenham
Copy link
Member

jorenham commented Apr 21, 2025

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
Contributor Author

Rebased to fix conflicts.

@jorenham
Copy link
Member

jorenham commented May 1, 2025

needs another rebase

@DimitriPapadopoulos
Copy link
Contributor Author

Rebased to fix conflicts.

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