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

Circular dependencies in Python < 3.7 string annotations #453

Closed
crusaderky opened this issue Jul 13, 2019 · 8 comments
Closed

Circular dependencies in Python < 3.7 string annotations #453

crusaderky opened this issue Jul 13, 2019 · 8 comments

Comments

@crusaderky
Copy link

crusaderky commented Jul 13, 2019

When a function of module A needs to use and return an object of module B, but module B already imports module A, you will typically see this pattern in Python >=3.5.2, <3.7:

from typing import TYPE_CHECKING
if TYPE_CHECKING:
    # Circular dependency; module_b imports this module in its header
    from module_b import SomeClass

def f() -> "SomeClass":
    from module_b import SomeClass
    return SomeClass(1, 2, 3)

pyflakes 2.1.1 produces errors when parsing this file:

module_a.py:4: 'module_b.SomeClass' imported but unused
module_a.py:7: redefinition of unused 'SomeClass' from line 4

Notably, pyflakes does not complain when the same is rewritten using delayed annotation, which however requires Python 3.7:

from __future__ import annotations
from typing import TYPE_CHECKING
if TYPE_CHECKING:
    # Circular dependency; module_b imports this module in its header
    from module_b import SomeClass

def f() -> SomeClass:
    from module_b import SomeClass
    return SomeClass(1, 2, 3)
@asottile
Copy link
Member

I cannot reproduce on master or on pyflakes 2.1.1

$ ~/opt/venv/bin/pyflakes  --version
2.1.1 Python 3.6.8 on Linux
$ ~/opt/venv/bin/pyflakes t.py
$ echo $?
0
$ python3.6 -m pyflakes t.py
$ echo $?
0

@crusaderky
Copy link
Author

crusaderky commented Oct 1, 2019

Apologies, I should have tested my snippet - it turns out I oversimplified my real use case.
The issue is actually triggered by typing.Union:

from typing import Union, TYPE_CHECKING
if TYPE_CHECKING:
    # Circular dependency; module_b imports this module in its header
    from module_b import SomeClass


def f() -> Union["SomeClass", int]:
    from module_b import SomeClass
    return SomeClass(1, 2, 3)

pyflakes output:

t.py:4: 'module_b.SomeClass' imported but unused
t.py:8: redefinition of unused 'SomeClass' from line 4

Please reopen

@crusaderky
Copy link
Author

Interestingly, this triggers the issue:

def f() -> Union["SomeClass", int]:

but this doesn't:

def f() -> "Union[SomeClass, int]":

and, as said in the op, this doesn't either (but requires py37):

from __future__ import annotations
def f() -> Union[SomeClass, int]:

@asottile
Copy link
Member

asottile commented Oct 1, 2019

that's a dupe of #447 -- the workaround for now being to quote the entire annotation or use __future__-annotations

@asottile
Copy link
Member

@crusaderky would you like to review the solution in #479 to see if it fixes your problem for you?

@crusaderky
Copy link
Author

@asottile confirmed it solves the issue

@asottile
Copy link
Member

@crusaderky would you be able to code review the patch?

@crusaderky
Copy link
Author

@asottile
I have tested the PR against my POC above, and it fixes the issue, and against the xarray codebase, which makes extremely heavy use of quoted annotations, and it doesn't cause any regressions.
I'm afraid I don't know enough of the pyflakes internals to figure out if there is any subtle issue not being covered by just reading the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants