ruff icon indicating copy to clipboard operation
ruff copied to clipboard

False positive with F401 (imported but unused)

Open sbdchd opened this issue 2 years ago • 4 comments

minimal example (of some project code):

from typing import List

x = [] # type: List[str]
x.append("foo")
print(x)

This isn't a big deal since # type: List[str] can be replaced with # type: list[str] and mypy warns about List being undefined with the auto fixer removes the import

sbdchd avatar Jan 04 '23 02:01 sbdchd

Is this in reference to the type comment? Like, that # type: List[str] should be considered a valid reference to List? Just verifying.

charliermarsh avatar Jan 04 '23 02:01 charliermarsh

Yeah exactly! mypy will complain that it isn't defined if the import is missing, i.e.

kodiak/pull_request.py:76:5: error: Name "List" is not defined  [name-defined]
kodiak/pull_request.py:76:5: note: Did you forget to import it from "typing"? (Suggestion: "from typing import List")

sbdchd avatar Jan 04 '23 02:01 sbdchd

Yeah we don't really support type comments right now, since they were made obsolete in Python 3.6 AFAIK. I know some projects still use them for compatibility... I'll think on it.

charliermarsh avatar Jan 04 '23 16:01 charliermarsh

@charliermarsh maybe 1 area that this is still useful in Python 3.6+: type hinting the result of a context manager. Example:

from types_aiobotocore_s3 import S3ServiceResource

async with self.boto_session.resource("s3") as s3:  # type: S3ServiceResource
    obj = await s3.Object(self.bucket_name, key)
    return (await obj.get())["Body"]

The from types_aiobotocore_s3 import S3ServiceResource part is removed by ruff.

Looks like leaving it off or on is ok by Mypy, but type hinting at least in Pycharm is better with the type comment.

phillipuniverse avatar Jan 27 '23 17:01 phillipuniverse

Maybe the issue title can be change to False positive with F401 (imported but unused) with mypy type comments ? can faster be found when searching for this issue.

Affects me as well.

spaceone avatar Feb 06 '23 11:02 spaceone

+1 would also be very helpful to be able to use type comments. IMO they're much nicer than normal type annotations in a lot of places, because they can "hide" as a comment out of site of the actual line of code

image

jaymegordo avatar Apr 21 '23 18:04 jaymegordo

I’d say the comment looking nicer than a type annotation is an editor/highlighter problem, but the general point of this being useful is correct.

uranusjr avatar Apr 24 '23 22:04 uranusjr

Similar to https://github.com/charliermarsh/ruff/issues/1619#issuecomment-1406845028, I still use type comments in some very rare cases where things like for loops don't have syntax to express a "real" Python 3 type hint and the type checkers can't infer the type due to limitations in the underlying libraries being used.

from models import Wheel

for wheel in car.wheels.all():  # type: Wheel
    ...

I realize this is non-standard, but PyCharm will interpret this and provide intellisense/auto-complete for the loop variable, which is practically useful in these niche cases.

johnthagen avatar May 01 '23 16:05 johnthagen

All type comments can be replaced (c.f.: https://peps.python.org/pep-0526/#where-annotations-aren-t-allowed)

from types_aiobotocore_s3 import S3ServiceResource

async with self.boto_session.resource("s3") as s3:  # type: S3ServiceResource
    obj = await s3.Object(self.bucket_name, key)
    return (await obj.get())["Body"]

# Becomes
s3: S3ServiceResource
async with self.boto_session.resource("s3") as s3:
    obj = await s3.Object(self.bucket_name, key)
    return (await obj.get())["Body"]
from models import Wheel

for wheel in car.wheels.all():  # type: Wheel
    ...
# Becomes
wheel: Wheel
for wheel in car.wheels.all():
    ...

JonathanPlasse avatar May 22 '23 08:05 JonathanPlasse

@JonathanPlasse Good point bringing that up. I've generally been a little resistant to this particular fix because it complicates the actual code just to add the hint:

  • It creates a new unbound local (if the code gets refactored later it could open up the possibility that s3: S3ServiceResource gets "separated" from the loop accidentally)
  • Python leaks loop variables into the outer scope already, but the new s3: S3ServiceResource could be misunderstood as implying intent to leak the iterator
  • There is a DRY issue where if the loop variable s3 is renamed but the outer typed variable isn't

But you're right that this is the officially prescribed way of dealing with it, but hopefully the above points help explain why people may be hesitant to do it just for a hint.

johnthagen avatar May 22 '23 11:05 johnthagen

Also, it's worth mentioning that if you unpack values in this manner:

n: str
p: Tensor
r: Tensor
f1: Tensor
for n, p, r, f1 in zip(names, precision, recall, f1_micro):
    ...

it seems unnecessarily complicated for such a simple functionality compared to a simpler approach like:

for n, p, r, f1 in zip(names, precision, recall, f1_micro):  # type: str, Tensor, Tensor, Tensor
    ...

V3RGANz avatar May 24 '23 11:05 V3RGANz

I stumbled onto this issue in the configparser project. I worked around it by converting the type comments to native type hints (jaraco/configparser@4990ba132450e218d9695ba74d4a139913459b17).

jaraco avatar Jul 08 '23 03:07 jaraco

Yeah we don't really support type comments right now, since they were made obsolete in Python 3.6 AFAIK. I know some projects still use them for compatibility... I'll think on it.

They are not obsolete, they are part of PEP 484 specification and are supported by all major type checkers (pyright, mypy, pyre, etc) https://peps.python.org/pep-0484/#type-comments

There are a ton of projects that leverage them, i.e. numpy.

vors avatar Jul 24 '23 18:07 vors

To be clear, "obsolete" is different from "unused". I'm not claiming that type comments aren't used in existing projects or that type checkers do not support them. It would be nice to support them! But it's a matter of prioritization, and my understanding is that PEP 526 had the explicit goal of introducing "a more readable syntax to replace type comments."

Perhaps another way of framing my lack of urgency to prioritize this: is there any reason to use type comments when writing Python code today? (This is not a rhetorical question, I am genuinely asking.)

charliermarsh avatar Jul 24 '23 18:07 charliermarsh

@charliermarsh personally I use them exclusively, (see my previous comment https://github.com/astral-sh/ruff/issues/1619#issuecomment-1518178907), purely because I think they make code much more readable. But thats just my 2c 🤷🏼‍♂️

jaymegordo avatar Jul 24 '23 18:07 jaymegordo

@charliermarsh

is there any reason to use type comments when writing Python code today?

As explained in the link below, for the rare cases of needing to type hint loop variables or with block variables, I find type comments avoid several drawbacks:

  • https://github.com/astral-sh/ruff/issues/1619#issuecomment-1557012758

johnthagen avatar Jul 25 '23 19:07 johnthagen

is there any reason to use type comments when writing Python code today?

The company where I work has multi-million line codebase and we just migrated to using ruff as THE linter. (thank you for the great project!)

It's not possible to migrate the whole codebase in a one go to the py3 comments unfortunately (there are technical reasons why we cannot do that).

vors avatar Jul 26 '23 02:07 vors

(Thank you, these comments are all welcome and helpful.)

charliermarsh avatar Jul 26 '23 02:07 charliermarsh

FWIW flake8 has the same problem (tested on flake8 6.1.0)

vors avatar Sep 15 '23 01:09 vors

pyflakes (what Flake8 runs under the hood) used to support this (I used the feature for years before moving to Ruff). Perhaps there was a regression.

  • https://github.com/PyCQA/pyflakes/pull/400

johnthagen avatar Sep 15 '23 01:09 johnthagen

It looks like it was removed about a year ago: https://github.com/PyCQA/pyflakes/pull/684

charliermarsh avatar Sep 15 '23 01:09 charliermarsh