search

acdh-oeaw / rdfproxy

GNU General Public License v3.0
0 stars 0 forks source link

Lupl/ruff include f401 #37

Closed lu-pl closed 3 weeks ago

lu-pl commented 1 month ago

Is the commit order now correct? Tests should work for a3cdfbf by itself, but not for 24d11a6.

lu-pl commented 1 month ago

Please replace the per file ignore with targeted inline comments:

# noqa: F401

What is the rationale for noqa over per-file-ignores?

A possible benefit of per-file-ignores is that IDE linters can still flag the unused imports, but ruff check and pipelines will pass.

b1rger commented 1 month ago

What is the rationale for noqa over per-file-ignores?

It is targeted at that specific line, instead of a broad catchall rule that might in the future ignore stuff we don't want ignored

lu-pl commented 1 month ago

What is the rationale for noqa over per-file-ignores?

It is targeted at that specific line, instead of a broad catchall rule that might in the future ignore stuff we don't want ignored

I see that per-line-ignores are generally a good idea (explicit is better than implicit!), but __init__.py as the public library interface imo is an exception to that. Every import line will always have to be explicitly ignored in that case, because no symbol will be used in __init__.py (apart from being imported).

It is of course possible that at some point actual logic will run in __init__.py, in which case explicit per-line-ignores are strongly indicated. As soon as a symbol is actually used, F401 must not be ignored on a file level.

b1rger commented 1 month ago

It is of course possible that at some point actual logic will run in __init__.py, in which case explicit per-line-ignores are strongly indicated. As soon as a symbol is actually used, F401 must not be ignored on a file level.

At that point, in n years, we will have totally forgotten about that one setting we have in pyproject.toml. So lets please not postpone doing the right thing to some unspecified point in the future