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

No cyclic-import messages with jobs=0 #4171

Closed
udifuchs opened this issue Mar 2, 2021 · 8 comments · Fixed by #8807 or #9093
Closed

No cyclic-import messages with jobs=0 #4171

udifuchs opened this issue Mar 2, 2021 · 8 comments · Fixed by #8807 or #9093

Comments

@udifuchs
Copy link
Contributor

udifuchs commented Mar 2, 2021

Running pylint -jobs=0 does not report about cyclic-import. This can be reproduced with a trivial example.
a.py:

import b

b.py:

import a

This issue is not new to pylint 2.7. It is more noticeable only because running jobs in parallel became more useful in this version.

pylint --version
pylint 2.7.1
astroid 2.5
Python 3.8.6 (default, Sep 25 2020, 09:36:53) 
[GCC 10.2.0]
@doublethefish
Copy link
Contributor

This is issue is mentioned in #374 .

@udifuchs
Copy link
Contributor Author

udifuchs commented Mar 4, 2021

Sorry, I missed this bug report.
I will close this issue.

@udifuchs udifuchs closed this as completed Mar 4, 2021
@doublethefish
Copy link
Contributor

It might be good to keep this open as it is a simpler repro and the other bug contains a lot of stuff.

@udifuchs udifuchs reopened this Mar 22, 2021
@hippo91
Copy link
Contributor

hippo91 commented Apr 9, 2021

@udifuchs thanks for the report.

@praiskup
Copy link

The problem with this trivial example is that it actually detects a non-issue. It seems completely fine to cross-include the modules, as long as the "use" (accessing objects from the module) happens on runtime (not import time).

@doublethefish
Copy link
Contributor

doublethefish commented Jun 29, 2022

The problem with this trivial example is that it actually detects a non-issue. It seems completely fine to cross-include the modules, as long as the "use" (accessing objects from the module) happens on runtime (not import time).

Whilst I agree that, in practical terms, the trivial example should "work", I would state that any form of cyclic import is either problematic or will become problematic. That said, if it were possible to detect use-at-import that would be a better, more complete check - but, I suspect that is almost impossible to do thoroughly because of aliasing.

Simpler and more useful approaches (vs use-at-import detection) would be to either:

  1. scope the import:
def run_time_use():
   """Safer but uglier.

   Still suffers from the potential aliasing problem, but is explicit and scoped, so the problem is contained."""
   import A
   a.do_something()
  1. explicitly disable the warning
"""Dangerous as use-at-import can be introduced later, and deployed to prod"""
import A  # pylint: disable=cyclic-import

@Pierre-Sassoulas
Copy link
Member

Closing as duplicate of #374, I added the example in a comment there.

@Pierre-Sassoulas Pierre-Sassoulas added the Duplicate 🐫 Duplicate of an already existing issue label Jul 1, 2022
@jacobtylerwalls
Copy link
Member

It might be good to keep this open as it is a simpler repro and the other bug contains a lot of stuff.

Indeed! Reopening because I'm about to mark #374 closed by a separate PR.

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