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

Pylint reports ungrouped-imports for TYPE_CHECKING imports #3382

Closed
Martmists-GH opened this issue Feb 3, 2020 · 4 comments · Fixed by #4702
Closed

Pylint reports ungrouped-imports for TYPE_CHECKING imports #3382

Martmists-GH opened this issue Feb 3, 2020 · 4 comments · Fixed by #4702
Labels
Needs reproduction 🔍 Need a way to reproduce it locally on a maintainer's machine

Comments

@Martmists-GH
Copy link

Steps to reproduce

from typing import TYPE_CHECKING
from anyio import run, connect_tcp, create_task_group

if TYPE_CHECKING:
    from typing import List
    from anyio import SocketStream, TaskGroup  # pylint: disable=ungrouped-imports

Remove the pylint: disable comment and it will report ungrouped-imports

Current behavior

ungrouped-imports is reported for any duplicate import sources defined in an if TYPE_CHECKING block, not including the typing module.

Expected behavior

Pylint accepts the syntax as it's common to have certain imports in TYPE_CHECKING if not needed at runtime.

pylint --version output

pylint 2.4.4
astroid 2.3.3
Python 3.8.1 (default, Jan  8 2020, 23:09:20) 
[GCC 9.2.0]
@PCManticore
Copy link
Contributor

Sorry, I'm not sure I'm able to reproduce this.
I tried with the following version and received the following output for the script without the disable itself. Not sure if there's something I'm missing.. Can you do a quick check to see what isort version you are using? We're relying on this library to figure out where the imports should be ordered.

$ python -m pylint --version
pylint 2.4.4
astroid 2.3.3
Python 3.8.0 (default, Nov  6 2019, 18:25:08)
[Clang 8.1.0 (clang-802.0.42)]
$ python -m pylint a.py
a.py:1:0: C0114: Missing module docstring (missing-module-docstring)
a.py:3:0: W0611: Unused run imported from anyio (unused-import)
a.py:3:0: W0611: Unused connect_tcp imported from anyio (unused-import)
a.py:3:0: W0611: Unused create_task_group imported from anyio (unused-import)

------------------------------------------------------------------
Your code has been rated at 2.00/10 (previous run: 2.00/10, +0.00)

@PCManticore PCManticore added the Needs reproduction 🔍 Need a way to reproduce it locally on a maintainer's machine label Feb 4, 2020
@Martmists-GH
Copy link
Author

Isort reports version VERSION 4.3.21
I can send a full file with the errors later.

@developedby
Copy link

Just tried reproducing this.

With the following code

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from logging import Logger
    from typing import List

pylint reports ungrouped-imports.
But if I change the order of the last two lines like so

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from typing import List
    from logging import Logger

Then no error is reported. The same applies to other imported modules; When the last import before TYPE_CHECKING is from the same module as the first import inside the if, then no error is reported, otherwise I get ungrouped-imports and the same applies to the typing module.

Versions:

  • pylint 2.4.4
  • astroid 2.3.0
  • Python 3.7.4
  • isort 4.3.21

@Apakottur
Copy link

Looking at the example given by @developedby, I think it's not a bug, but how pylint behaves.
On one hand the imports should be grouped by modules, and on the other the TYPE_CHECKING clause should be at the end (otherwise the wrong-import-position) error is raised.
I see two solutions, but I'm not even sure which one is more "Pythonic":

  1. Put the if TYPE_CHECKING check in the middle of the import list:
from typing import TYPE_CHECKING
if TYPE_CHECKING:
    from typing import List

 from logging import Logger

In this case, the behavior of the checker wrong-import-position needs to be modified, to allow if TYPE_CHECKING between imports.
2. Keep the code as in the example:

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from logging import Logger
    from typing import List

In this case, the behvaior of the checker ungrouped-imports should be modified to consider the two import clauses (one outside the if and one inside it) separately.
In that case, we'd require that there would only be a single if TYPE_CHECKING in the code.

Another alternative is to use # pylint: disable, but I personally believe that checkers should be either on or off, and developers in my team should not add local disables in the code, which tend to become an easier way of passing linter checkers (instead of fixing the code).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs reproduction 🔍 Need a way to reproduce it locally on a maintainer's machine
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants