Skip to content
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

F401 unused-import for star-imports: false negative? #2407

Closed
spaceone opened this issue Jan 31, 2023 · 8 comments
Closed

F401 unused-import for star-imports: false negative? #2407

spaceone opened this issue Jan 31, 2023 · 8 comments
Labels
wontfix This will not be worked on

Comments

@spaceone
Copy link
Contributor

flake8 and ruff differ for * imports:

$ flake8 - <<<"from foo import *"
stdin:1:1: F403 'from foo import *' used; unable to detect undefined names
stdin:1:1: F401 'foo.*' imported but unused
$ ruff - <<<"from foo import *"
-:1:1: F403 `from foo import *` used; unable to detect undefined names
Found 1 error.

I detected this as prior it had a # noqa: F401 comment which was removed by RUF100 when I compared any leftovers with flake8.

@spaceone
Copy link
Contributor Author

I filed an upstream bug: PyCQA/pyflakes#766
I would vote for fixing it in pyflakes.

@spaceone
Copy link
Contributor Author

So they think that if F405 doesn't occur they mark the star import as F401.

F405 can be triggered by:

$ python3 -m pyflakes <<< $'from foo import *; __all__ = ("a",)'
<stdin>:1:1: 'from foo import *' used; unable to detect undefined names
<stdin>:1:20: 'a' may be undefined, or defined from star imports: foo
$ python3 -m pyflakes <<< $'from foo import *; a'
<stdin>:1:1: 'from foo import *' used; unable to detect undefined names
<stdin>:1:20: 'a' may be undefined, or defined from star imports: foo

@charliermarsh
Copy link
Member

I think I'm okay with Ruff's behavior here but open to being convinced otherwise. What do you think?

@charliermarsh charliermarsh added the question Asking for support or clarification label Jan 31, 2023
@spaceone
Copy link
Contributor Author

I am unsure. I like ruffs behavior more. Will think what I can do for the time I need to support both flake8 and ruff (aka: until #2402 is fixed).

@ngnpope
Copy link
Contributor

ngnpope commented Jan 31, 2023

For a file containing just the following (which is often seen in __init__.py):

from math import *
flake8 bug.py
bug.py:1:1: F403 'from math import *' used; unable to detect undefined names
bug.py:1:1: F401 'math.*' imported but unusedruff bug.py 
bug.py:1:1: F403 `from math import *` used; unable to detect undefined names
Found 1 error.

Using something from the math module:

from math import *
pi
flake8 bug.py
bug.py:1:1: F403 'from math import *' used; unable to detect undefined names
bug.py:2:1: F405 'pi' may be undefined, or defined from star imports: mathruff bug.py 
bug.py:1:1: F403 `from math import *` used; unable to detect undefined names
bug.py:2:1: F405 `pi` may be undefined, or defined from star imports: `math`
Found 2 errors.

Using something that is not in the math module:

from math import *
invalid_name
flake8 bug.py
bug.py:1:1: F403 'from math import *' used; unable to detect undefined names
bug.py:2:1: F405 'invalid_name' may be undefined, or defined from star imports: mathruff bug.py 
bug.py:1:1: F403 `from math import *` used; unable to detect undefined names
bug.py:2:1: F405 `invalid_name` may be undefined, or defined from star imports: `math`
Found 2 errors.

It has always bugged me getting F401 and F403 when the latter feels like it gives the expectation of the former, so I do quite like the way ruff does this... But I wouldn't be upset if it was implemented like the original.

@charliermarsh
Copy link
Member

Going to close for now because I'm comfortable with our current behavior.

@charliermarsh charliermarsh added wontfix This will not be worked on and removed question Asking for support or clarification labels Feb 2, 2023
@enriquebos
Copy link

I think if all is defined it should recognize the star import, only throw if all is also undefined.

@niniack
Copy link

niniack commented Mar 2, 2024

I think if all is defined it should recognize the star import, only throw if all is also undefined.

I think I agree with this, but it would be good to hear if someone thinks there is a reason why this is bad practice for __init__.py files. Anyways, it was really bothering me so I used the following in my pyproject.toml, adapted from here

[tool.ruff.lint.per-file-ignores]
"__init__.py" = ["F401", "F403"]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

5 participants