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

False-positive unused-import warnings with custom objects comment type annotations #3178

Closed
pandrey-fr opened this issue Oct 10, 2019 · 3 comments
Labels
Bug 🪲 Good first issue Friendly and approachable by new contributors

Comments

@pandrey-fr
Copy link

pandrey-fr commented Oct 10, 2019

Steps to reproduce

  • Lint this first script. Everything goes well.
# coding: utf-8

"""Example script - workable case."""

from typing import Callable, Dict, Tuple

def some_closure() -> Tuple[Callable[[str], int], Callable[[str, int], None]]:
    """Toy closure example."""
    mydict = {}  # type: Dict[str, int]
    def read_value(key: str) -> int:
        """Read a value from the state dict."""
        nonlocal mydict
        return mydict[key]
    def write_value(key: str, value: int) -> None:
        """Set a value in the state dict."""
        nonlocal mydict
        mydict[key] = value
    return read_value, write_value
  • Lint this modified script. Witness W0611: Unused numpy imported as np (unused-import)
# coding: utf-8

"""Example script - warning-triggering case."""

from typing import Callable, Dict, Tuple, Union

import numpy as np  # type: ignore

def some_closure() -> Tuple[Callable[[str], int], Callable[[str, int], None]]:
    """Toy closure example."""
    mydict = {}  # type: Dict[str, Union[int, np.ndarray]]
    def read_value(key: str) -> int:
        """Read a value from the state dict."""
        nonlocal mydict
        return mydict[key]
    def write_value(key: str, value: int) -> None:
        """Set a value in the state dict."""
        nonlocal mydict
        mydict[key] = value
    return read_value, write_value

Current behavior

In the first example, pylint analyzes the comment type annotation correctly, and does not complain about typing.Dict being imported (it would if the comment was removed, but then mypy would complain about the lack of type annotation for the empty-initialized dict).

In the second example, pylint still analyzes the comment type annotation and does not complain about Dict and Union being imported from typing, however it does complain about the numpy import, whereas the latter is also done so as to specify the type annotation.

This toy example may seem irrelevant (and indeed, the type annotation is here incorrect), but it emerges in my actual code when using imports within a package (e.g. the function used to add values in a dict attribute is imported from another code file, and returns a custom type ; I therefore import said type to type-annotate the dict which is bound to be assigned values at some point, and get pylint warnings).

Expected behavior

Ideally, pylint would detect third-party types imports used in type annotations, in addition to the basic typing types. Of course, warnings can be disabled by adding pylint comments in code files, which might be the right solution if implementing a fix proves too hard, but I still thought it was worth signalling.

pylint --version output

pylint 2.4.2
astroid 2.3.1
Python 3.6.8 (default, Oct  7 2019, 12:59:55) 
[GCC 8.3.0]
@PCManticore
Copy link
Contributor

Thanks for raising the issue, I think it makes sense to mimick the same behaviour for third party libraries.

@PCManticore PCManticore added Bug 🪲 Good first issue Friendly and approachable by new contributors labels Oct 11, 2019
@DaniGuardiola
Copy link

DaniGuardiola commented Feb 26, 2020

@PCManticore hi :)

This still fails when using the type in a string, for example:

import SomeClass

def func(some_instance: "SomeClass"):
	pass

I'm using pylint 2.4.4

@pandrey-fr
Copy link
Author

@DaniGuardiola It seems that the issue was fixed on master branch by @PCManticore (thanks!), however this was done after the 2.4.4 release - hence it should be included in the next version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🪲 Good first issue Friendly and approachable by new contributors
Projects
None yet
Development

No branches or pull requests

3 participants