mypy icon indicating copy to clipboard operation
mypy copied to clipboard

Typeshed import cycle causes mypy to start claiming a fixed-length tuple type alias declared with `TypeAlias` is "not valid as a type"

Open AlexWaygood opened this issue 2 years ago • 7 comments

Bug Report

We're having a bit of trouble over in https://github.com/python/typeshed/pull/11074. It seems like it's impossible to import importlib.readers inside stdlib/importlib/machinery.pyi and also use typing_extensions.TypeAlias to explicitly demarcate type aliases inside stdlib/zipfile.pyi. The cause appears to be a huge import cycle in our stubs for the standard library:

  • importlib.machinery imports NamespaceReader from importlib.resources.readers
  • importlib.reasources.readers imports zipfile.Path from zipfile
  • zipfile imports TypeAlias from typing_extensions
  • typing_extensions imports get_original_bases from types
  • types imports ModuleSpec from importlib.machinery... and we're back at the beginning.

As a workaround, things seem to work fine for us if we just don't use typing_extensions.TypeAlias for the problematic alias in zipfile.pyi. But this behaviour from mypy seems buggy, so I figured it would be good to file a bug.

To Reproduce

I haven't been able to reproduce this issue outside of the typeshed context. However, I have reduced typeshed down to a "minimum viable typeshed" required to reproduce the bug (https://github.com/AlexWaygood/typeshed/tree/mypy-import-cycle-repro/stdlib). Here are the repro steps:

  1. Clone https://github.com/AlexWaygood/typeshed
  2. Checkout the mypy-import-cycle-repro branch of the repo
  3. Create and activate a venv; run pip install -r requirements-tests.txt
  4. Run python tests/mypy_test.py stdlib -p3.12

Expected Behavior

Mypy handles the import cycle -- or at least gives an intelligible error reporting what the problem is.

Actual Behavior

stdlib\zipfile.pyi:4: error: Variable "zipfile._DateTuple" is not valid as a type  [valid-type]
stdlib\zipfile.pyi:4: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases

Your Environment

  • Mypy version used: 1.7.1
  • Mypy command-line flags: https://github.com/AlexWaygood/typeshed/blob/4235e8c82a0378b7ce5df878659ed0e909692f13/tests/mypy_test.py#L252-L275
  • Python version used: 3.12

AlexWaygood avatar Nov 28 '23 18:11 AlexWaygood

Are you suggesting that mypy can handle any cyclic import situation? (I can't find docs on that, but plenty of issues that are still open.)

My hunch is to establish a hierarchy and not have low level import from high level, and break the cycle. (BTW, does the same cycle not manifest in the runtime modules?)

ikonst avatar Nov 29 '23 05:11 ikonst

It should yes, since it doesn’t have to actually execute the code at any point. At runtime this isn’t an issue since among other things zipfile doesn’t actually need TypeAlias.

TeamSpen210 avatar Nov 29 '23 05:11 TeamSpen210

Cyclic dependency between types can occur even if you don't execute. Looking at mypy issues, there seem to be issues having to do with cyclic imports. Does mypy fare just slightly better than runtime by doing multiple passes?

a.py:

from b import B
A = B

b.py:

from a import A
B = A

ikonst avatar Nov 29 '23 06:11 ikonst

Are you suggesting that mypy can handle any cyclic import situation? (I can't find docs on that, but plenty of issues that are still open.)

No, of course it would be unreasonable to expect it to handle any arbitrary cyclic import. But we've had complex cyclic imports in typeshed for a long time now, and it would be pretty difficult to write stdlib stubs that avoided them! This, unfortunately, just seems to be a step too far.

Anyway, even if we decide there is a line where the cyclic imports get too complex for mypy to handle — mypy should have a better error message here that tells the user that cyclic imports are the problem! Randomly reporting that a type alias in an unrelated file is no longer "valid as a type" is not great UI :)

At runtime this isn’t an issue since among other things zipfile doesn’t actually need TypeAlias.

Yeah. And types doesn't actually import ModuleSpec from importlib.machinery at runtime, but we need to do so in order to annotate things in types.pyi in the stubs. The fact that I can't repro outside the typeshed context also suggests that the builtins/typing import cycle in typeshed might somehow be involved as well.

AlexWaygood avatar Nov 29 '23 10:11 AlexWaygood

Would this kinda fix itself (at least for stubs) once all major type-checkers fully support the new type syntax? Since importing TypeAlias wouldn't be necessary anymore. Not sure how likely this is to happen in .py files outside the stdlib anyway.

Avasam avatar Aug 19 '24 17:08 Avasam

Would this kinda fix itself (at least for stubs) once all major type-checkers fully support the new type syntax?

Maybe, but we won't be dropping support for Python 3.11 for a long time in typeshed, and we can't switch to the new syntax until we do so. This is because the Python stdlib ast module literally won't parse the new syntax as valid syntax before then, so mypy can't even start analysing stubs if you're running mypy with Python 3.11 and the stubs are using py312+ syntax

AlexWaygood avatar Aug 19 '24 17:08 AlexWaygood

Ah so the same problem we had with parsing positional-only marker / on Python 3.7

Avasam avatar Aug 19 '24 17:08 Avasam