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

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

Open
AlexWaygood opened this issue Nov 28, 2023 · 4 comments
Labels
affects-typeshed Anything that blocks a typeshed change bug mypy got something wrong topic-import-cycles topic-type-alias TypeAlias and other type alias issues

Comments

@AlexWaygood
Copy link
Member

AlexWaygood commented Nov 28, 2023

Bug Report

We're having a bit of trouble over in python/typeshed#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

@AlexWaygood AlexWaygood added bug mypy got something wrong topic-import-cycles topic-type-alias TypeAlias and other type alias issues affects-typeshed Anything that blocks a typeshed change labels Nov 28, 2023
@AlexWaygood AlexWaygood changed the title Typeshed import cycle causes mypy to start claiming a fixed-length tuple annotated with TypeAlias is "not valid as a type" Typeshed import cycle causes mypy to start claiming a fixed-length tuple type alias declared with TypeAlias is "not valid as a type" Nov 28, 2023
@ikonst
Copy link
Contributor

ikonst commented Nov 29, 2023

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?)

@TeamSpen210
Copy link
Contributor

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.

@ikonst
Copy link
Contributor

ikonst commented Nov 29, 2023

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

@AlexWaygood
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-typeshed Anything that blocks a typeshed change bug mypy got something wrong topic-import-cycles topic-type-alias TypeAlias and other type alias issues
Projects
None yet
Development

No branches or pull requests

3 participants