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

cyclic-import warning detects a non-issue situation #6983

Closed
praiskup opened this issue Jun 20, 2022 · 7 comments
Closed

cyclic-import warning detects a non-issue situation #6983

praiskup opened this issue Jun 20, 2022 · 7 comments
Labels
Duplicate 🐫 Duplicate of an already existing issue Invalid Not a bug, already exists or already fixed

Comments

@praiskup
Copy link

Bug description

shell>$ ls pylint_mod/
a.py  b.py  __init__.py

shell>$ cat pylint_mod/a.py 
import pylint_mod.b

def x():
    print("In A module")

def test():
    pylint_mod.b.x()

shell>$ cat pylint_mod/b.py 
import pylint_mod.a

def x():
    print("In B module")

def test():
    pylint_mod.a.x()

The problem is that this warning is currently printed by pylint:

$ pylint pylint_mod/a.py pylint_mod/b.py  | grep -i cycl
pylint_mod/b.py:1:0: R0401: Cyclic import (pylint_mod.a -> pylint_mod.b) (cyclic-import)

While this technically is a cyclic-import, practically it should be a non-issue. Both the cyclic cross-module uses are delayed to the run-time, therefore all the module objects are correctly loaded when really needed.

Configuration

No response

Command used

pylint pylint_mod/a.py pylint_mod/b.py

Pylint output

pylint_mod/b.py:1:0: R0401: Cyclic import (pylint_mod.a -> pylint_mod.b) (cyclic-import)

Expected behavior

No error printed.

Pylint version

pylint 2.13.7
astroid 2.11.5

OS / Environment

No response

Additional dependencies

No response

@praiskup praiskup added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Jun 20, 2022
@Pierre-Sassoulas
Copy link
Member

cyclic-import is a refactor message, not an error message it's acting as intended in this case. (i.e. it's a style check, it's not supposed to warn you only of something that will crash). It's possible to disable the check if you don't like it.

@Pierre-Sassoulas Pierre-Sassoulas added Won't fix/not planned Invalid Not a bug, already exists or already fixed and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling Won't fix/not planned labels Jun 20, 2022
@praiskup
Copy link
Author

The thing is that we have to turn this warning ON or OFF, only because of this issue. While typically it provides useful info.

@Pierre-Sassoulas
Copy link
Member

See #850

@Pierre-Sassoulas Pierre-Sassoulas added the Duplicate 🐫 Duplicate of an already existing issue label Jun 20, 2022
@praiskup
Copy link
Author

#850 is similar, but not the same. We don't want to import inside functions. This is a legitimate import at the file top level.

@Pierre-Sassoulas
Copy link
Member

As I said this message is about style we're not going to change it. You need to be able to disable it locally though (in a function or elsewhere) without disabling for the whole project.

@praiskup
Copy link
Author

You need to be able to disable it locally though (in a function or elsewhere) without disabling for the whole project.

To be able to do this, we'd have to switch to a function-local import, which is something we don't want to.
Therefore we have to disable this for the whole project.

@DanielNoord
Copy link
Collaborator

@praiskup What @Pierre-Sassoulas is talking about is that we should fix cyclic-import to be disable-able per line/module/scope.

It's in the spirit of the message to report on cases such as these, and as explained in the linked message it doesn't make a lot of sense to change the spirit of the message.
That said, the message is not as controllable as we would like and how we allow control for other messages. That's something that should be worked on.

After that has been merged you should be able to add a disable to module/lines where you want to use this pattern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate 🐫 Duplicate of an already existing issue Invalid Not a bug, already exists or already fixed
Projects
None yet
Development

No branches or pull requests

3 participants