GeezerVison

Instead of blanket everything # noqa comments, please make these specific to the thing being disabled. # noqa: unused-import for example. Except that ruff appears to not support human meaningful and thus maintainable suppression comments in favor of inhumane error code numbers in noqa comments? So that might read F401 today.

But at least we could fix those when ruff enters the modern era and names things to be understandable by code maintainers.

Instead of blanket everything # noqa comments, please make these specific to the thing being disabled.

On a line like import readline # noqa, it's clear that noqa suppress a warning about unused import, no?

So that might read F401 today.

So you suggest using # noqa: F401 instead?

Except that ruff appears to not support human meaningful and thus maintainable suppression comments in favor of inhumane error code numbers in noqa comments? So that might read F401 today.

But at least we could fix those when ruff enters the modern era and names things to be understandable by code maintainers.

it's on our to-do list :-)

Edit: and I see Zanie already said that on the Ruff issue tracker ;)

yep, always list the specific error(s) being ignored in the comment.

How did you know that all these imports were necessary? Or did you just flag all the names for which you can't confirm redundant imports?

I'm checking for unused imports for 5 years, and every year, I meet the same imports :-) Over time, I learnt which imports are done to populate an API, and which ones can be removed.

You can try to remove them and see the test suite failing.

I agree with all this, I just wondering how did you find them, because the amount of the work is large, and many cases are not trivial. Removing imports unlit the code breaks is risky, not all can be covered by tests.

I modified the PR to use # noqa: F401. Hopefully, it seems like warning numbers are standardized between linters.

I agree with all this, I just wondering how did you find them, because the amount of the work is large, and many cases are not trivial. Removing imports unlit the code breaks is risky, not all can be covered by tests.

I also rely on code review to validate my work :-)

yep, always list the specific error(s) being ignored in the comment.

There's a rule (PGH004 blanket-noqa) to prevent noqas that are missing the specific error. We have it in https://github.com/python/cpython/blob/main/Tools/clinic/.ruff.toml:

Shall we add it to the other one, https://github.com/python/cpython/blob/main/Lib/test/.ruff.toml ?

And maybe add another .ruff.toml for Lib/ or other directories?

I'm checking for unused imports for 5 years, and every year, I meet the same imports :-) Over time, I learnt which imports are done to populate an API, and which ones can be removed.

If we added F401 unused-import to Ruff, these would get flagged by the CI, and we'd have to label the ones we really need with noqa: F401.

I created a second PR for the 7 files which requires tons of #noqa: #120454.

Merged, thanks for reviews!

@hugovk: I'm interested by a CI checking for unused imports, it's appealing :-)

For things that are re-exported for a public API, should add them to __all__ rather than add the linter directive?

For things that are re-exported for a public API, should add them to __all__ rather than add the linter directive?

usually IMO, yes