Pylint: Pylint detects imaginary cyclic imports

[pde]

If two modules import from each other, but one of them only imports the other within functions and other non-global scopes, Python is happy. But pylint still errors on such cyclic imports, and in fact won't accept a module-scoped disable directive, so cyclic import detection has to be disabled for the entire project.

Here's an example from this PR: https://github.com/letsencrypt/letsencrypt/pull/2649 :

git clone https://github.com/letsencrypt/letsencrypt
cd letsencrypt
git checkout split-cli
vim .pylintrc # remove the disable cyclic-import
tools/venv.sh
venv/bin/pylint --rcfile=.pylintrc letsencrypt/cli.py letsencrypt/main.py
************* Module letsencrypt.main
R:  1, 0: Cyclic import (letsencrypt.cli -> letsencrypt.main) (cyclic-import)
...

[The-Compiler]

I think the checker itself is working as intended - cyclic imports can be problematic even when they don't break right away (like when using from-imports instead, or on older Python versions - for example, 3.5 can handle some cyclic imports 3.4 can't).

That being said, I disabled it myself too because it was too noisy. Then again, it only complaining when Python complains anyways would be somewhat pointless.

Maybe as a middle ground, cyclic-import shouldn't get emitted when an import is inside a function, as then clearly the developer is aware they're working around a circular import? Let's see what @PCManticore thinks.

[PCManticore]

I think it makes sense not to emit the error in case one of the imports is inside a function scope, as @The-Compiler already said. Sometimes getting rid of cyclic imports is impossible, requiring redesigning the entire project (as we already have this problem in astroid for example). That being said, I'll probably work on a patch in this weekend, if no one else wants to do it.

[sthenault]

IMHO we should always warn about cyclic import and let the user explicitly
deactivate it rather than adding a specific rule.
Le 18 mars 2016 12:47, "Claudiu Popa" [email protected] a écrit :

I think it makes sense not to emit the error in case one of the imports is
inside a function scope, as @The-Compiler
https://github.com/The-Compiler already said. Sometimes getting rid of
cyclic imports is impossible, requiring redesigning the entire project (as
we already have this problem in astroid for example). That being said, I'll
probably work on a patch in this weekend, if no one else wants to do it.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
https://github.com/PyCQA/pylint/issues/850#issuecomment-198321536

[PCManticore]

True, but at least they should have some control over disabling the error in a more granular way. My initial thought is to have a flag that can control if the users want to not warn about cyclic-import if one of the imports is protected by a function scope, defaulting to false, so with the same behaviour as now.

[sthenault]

another idea would be that "# disable: cyclic-import" would make the
matched imports unconsidered the cycle computation, breaking cycle if any.

On Fri, Mar 18, 2016 at 7:13 PM, Claudiu Popa [email protected]
wrote:

True, but at least they should have some control over disabling the error
in a more granular way. My initial thought is to have a flag that can
control if the users want to not warn about cyclic-import if one of the
imports is protected by a function scope, defaulting to false, so with the
same behaviour as now.

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
https://github.com/PyCQA/pylint/issues/850#issuecomment-198478596

Sylvain

[rogalski]

Discussion looks like a duplicate of #59, with exception of function-scope imports.

[actionless]

mb it could be two different warnings, cyclic-import and cyclic-import-from-function ?

[ParasJain-dev]

@PCManticore Is this issue fixed?
We are still getting this issue in pylint==1.9.*