Cycle detected in import chain

[falan]

Describe the bug
Pyright report cycle detected in import chain when using the method described in the link.
https://mypy.readthedocs.io/en/stable/common_issues.html#import-cycles

To Reproduce
File foo.py:

from typing import List, TYPE_CHECKING

if TYPE_CHECKING:
    import bar

def listify(arg: 'bar.BarClass') -> 'List[bar.BarClass]':
    return [arg]

File bar.py:

from typing import List
from foo import listify

class BarClass:
    def listifyme(self) -> 'List[BarClass]':
        return listify(self)

Expected behavior
No errors.

VS Code extension or command-line
VS Code extension 1.1.44
Python 3.7.4

[erictraut]

This behavior is "as designed" in Pyright. If you enable the cycle detection feature, Pyright will detect and report all import cycles regardless of whether they actually result in an exception at runtime. Import cycles, even those that don't generate runtime exceptions because they are guarded by a runtime conditional, point to architectural layering issues that should be avoided in code. This feature is intended to detect and report these layering violations.

If you don't care about architectural layering violations, you can disable the cycle detection in Pyright by disabling this diagnostic rule.

If you do care about architectural layering, then you can refactor your code to eliminate the cycles. This might mean splitting up declarations into different source files — i.e. placing low-level constants, utility functions, and class definitions in a "leaf" module and placing higher-level classes and functions in a separate file that consumes the lower-level definitions.

[falan]

Thank you for your kind instruction.

[qwertey6]

@erictraut , What is the suggested solution for import cycles generated when creating self-referencing SQLalchemy models? Circular references are fine in DBMSs, however to statically type ORM models which capture these circular relationships, one would need to create self-referencing parent and child classes.

The only way of avoiding import cycles is currently to either not add types, or put all offending classes into a single file, which in my opinion feels like more of an "architectural layering violation" than cyclical type imports.

Import cycle detection is a wonderful feature, but would benefit greatly from understanding if TYPE_CHECKING conditionals. I was actually surprised that this wasn't the default behavior, as I'm coming from typescript (also in vscode), where import type {...} from 'SomeFileThatImportsMe'; is valid, whereas importing code causes a "real" import cycle. It would be great if pyright/pylance offered support for python's if TYPE_CHECKING flavor of import type

[erictraut]

The solution I use in this case is to place model classes in the same file if they have mutual recursive dependencies.

Use of conditional imports within TYPE_CHECKING conditional checks is not something I recommend in general. In fact, any use of TYPE_CHECKING is fragile because it means that the code that the type checker "sees" is different from the code that the interpreter "sees" at runtime. I realize that there are times when TYPE_CHECKING is the least-bad solution, but it's still a pretty bad solution. Placing model classes in the same file sounds like a much cleaner and less fragile solution.

Making the type checker's behavior dependent on whether code is found within a TYPE_CHECKING conditional block would further exacerbate the fragility problem and make it even more confusing to reason about what it's doing. For that reason, I would be very reluctant to make the import cycle generation "understand TYPE_CHECKING conditionals".