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

Decouple collections.abc types from typing aliases #6257

Open
cdce8p opened this issue Nov 8, 2021 · 11 comments
Open

Decouple collections.abc types from typing aliases #6257

cdce8p opened this issue Nov 8, 2021 · 11 comments
Labels
stubs: improvement Improve/refactor existing annotations, other stubs issues

Comments

@cdce8p
Copy link
Contributor

cdce8p commented Nov 8, 2021

At the moment, collections.abc types are strict aliases for their typing counterparts. This can make it difficult for intellisense providers to distinguish them and suggest the correct one. @erictraut mentioned that for pyright: microsoft/pylance-release#1001 (comment).

Although this would likely be a lot of code duplication in the short term, I propose to copy the type definitions to the _collections_abc.pyi module guarded by sys.version_info >= (3, 9).

@srittau
Copy link
Collaborator

srittau commented Nov 8, 2021

I'm fine with everything that brings typeshed closer to the implementation. But please note that generics are expected to work with collections.abc with all Python versions in stubs or when using the future import.

@cdce8p
Copy link
Contributor Author

cdce8p commented Nov 8, 2021

But please note that generics are expected to work with collections.abc with all Python versions in stubs or when using the future import.

Good point! The idea originally was to use the old aliases for < (3, 9), but that might not be necessary.

--
I tried implementing the change, but came across an issue. Decoupling the typing alias and the definition in _collections_abc also means that they aren't recognized as the same type anymore. That is especially an issue with builtin types like tuple. The base classes are usually the typing ones, e.g. typing.Sequence.

It's easy enough to change, but this will likely cause even more issues as tuple wouldn't inherit from typing.Sequence anymore.

Not quite sure how this can resolve if at all possible.

class tuple(Sequence[_T_co], Generic[_T_co]):

@JelleZijlstra
Copy link
Member

This is tricky and I think we'll need some special casing in type checkers either way. On the one hand, we want these types to be treated the same by the type checker for the most part: you should be able to pass a typing.Tuple to an argument annotated as tuple, or a typing.Sequence to a collections.abc.Sequence, and vice versa. But on the other hand, type checkers may want to distinguish them so they can warn correctly about runtime subscriptabiliity issues.

@erictraut
Copy link
Contributor

If we want type checkers to treat these specially, we may want to define them using _Alias(), like the other special-cased aliases in typing.pyi. Otherwise they look just like any other type alias definition.

@cdce8p
Copy link
Contributor Author

cdce8p commented Nov 8, 2021

I've opened #6262 to have some basis for a discussion. _Alias() is a good idea. It seems though that pyright, mypy, (and others ?) need some work to support it.

@erictraut
Copy link
Contributor

For pyright, the change is relatively simple (just updating some internal tables). Do you have a full list of affected symbols? Is it simply the subset of symbols listed in PEP 585 that begin with collections.abc?

@cdce8p
Copy link
Contributor Author

cdce8p commented Nov 9, 2021

Is it simply the subset of symbols listed in PEP 585 that begin with collections.abc?

Not quite. If I didn't miss any, these should be it.

# These should work already
typing.Type -> type
typing.Tuple -> tuple
typing.List -> list
typing.Dict -> dict
typing.Set -> set
typing.FrozenSet -> frozenset
typing.DefaultDict -> collections.defaultdict
typing.Counter -> collections.Counter
typing.Deque -> collections.Deque
typing.ChainMap -> collections.ChainMap
typing.OrderedDict -> collections.OrderedDict  # >= 3.7
# Probably not yet handled
typing.AbstractSet -> collections.abc.Set  # ! AbstractSet -> Set
typing.AsyncGenerator -> collections.abc.AsyncGenerator
typing.AsyncIterable -> collections.abc.AsyncIterable
typing.AsyncIterator -> collections.abc.AsyncIterator
typing.Awaitable -> collections.abc.Awaitable
typing.Collection -> collections.abc.Collection
typing.Container -> collections.abc.Container
typing.Coroutine -> collections.abc.Coroutine
typing.Generator -> collections.abc.Generator
typing.ItemsView -> collections.abc.ItemsView
typing.Iterable -> collections.abc.Iterable
typing.Iterator -> collections.abc.Iterator
typing.KeysView -> collections.abc.KeysView
typing.Mapping -> collections.abc.Mapping
typing.MappingView -> collections.abc.MappingView
typing.MutableMapping -> collections.abc.MutableMapping
typing.MutableSequence -> collections.abc.MutableSequence
typing.MutableSet -> collections.abc.MutableSet
typing.Reversible -> collections.abc.Reversible
typing.Sequence -> collections.abc.Sequence
typing.ValuesView -> collections.abc.ValuesView

typing.ByteString -> collections.abc.ByteString  # only 'collections.abc' variant is subscriptable
typing.Hashable -> collections.abc.Hashable  # not subscriptable
typing.Sized -> collections.abc.Sized  # not subscriptable

typing.Callable -> collections.abc.Callable

typing.Match -> re.Match  # >= 3.7
typing.Pattern -> re.Pattern  # >= 3.7

typing.ContextManager -> contextlib.AbstractContextManager  # ! Name
typing.AsyncContextManager -> contextlib.AbstractAsyncContextManager  # ! Name


# Deprecated since 3.8, will be removed in 3.12 (maybe not worth supporting?)
typing.re.Match -> re.Match  # >= 3.7
typing.re.Pattern -> re.Pattern  # >= 3.7

If I didn't mention a Python version explicitly, it should be supported in >= 3.6.2.

@cdce8p
Copy link
Contributor Author

cdce8p commented Nov 9, 2021

I just realized that my comment about the Python version can be a bit confusing and not really relevant to the issue. They are the versions where the classes became first available at runtime.

What's important

  • Runtime generic support was added in 3.9 (except for Hashable and Sized)
  • They should be subscriptable in string annotations, (PEP 563), type comments, stub files, regardless of the Python version

@cdce8p
Copy link
Contributor Author

cdce8p commented Nov 11, 2021

I'm having some issues implementing the required mypy changes.

The easy part is updating the type alias list here: mypy/nodes.py. The problem I've now come up against is that both builtins.str and builtins.list seem to be needed after just the first pass. The best I can tell they are dependent on Sequence though, thus they are only defined as PlaceholderNode instead of TypeInfo.

The two main areas of conflict:

  1. mypy/semanal.py -> add_implicit_module_args -> builtins.list
  2. mypy/semanal.py -> str_type -> builtins.str

Maybe someone has an idea, how to resolve it.

--
To reproduce the issue:

type_aliases: Final = {
    ...
    'typing.Sequence': 'collections.abc.Sequence',
}
type_aliases_source_versions: Final = {
    ...
    'typing.Sequence': (3, 3),
}
  • Run mypy with custom typeshed
from collections.abc import Sequence

def func(var: Sequence[str] = ("Hello",)) -> None:
    pass
mypy --custom-typeshed-dir ../typeshed/ --show-traceback --no-incremental test.py

--
The traceback I got

../typeshed/stdlib/collections/__init__.pyi: error: INTERNAL ERROR -- Please try using mypy master on Github:
https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
Please report a bug at https://github.com/python/mypy/issues
version: 0.920+dev.2db0511451ecf511f00dbfc7515cb6fc0d25119c.dirty
Traceback (most recent call last):
  ...
  File "/.../mypy/mypy/__main__.py", line 23, in <module>
    console_entry()
  File "/.../mypy/mypy/__main__.py", line 11, in console_entry
    main(None, sys.stdout, sys.stderr)
  File "/.../mypy/mypy/main.py", line 87, in main
    res, messages, blockers = run_build(sources, options, fscache, t0, stdout, stderr)
  File "/.../mypy/mypy/main.py", line 165, in run_build
    res = build.build(sources, options, None, flush_errors, fscache, stdout, stderr)
  File "/.../mypy/mypy/build.py", line 180, in build
    result = _build(
  File "/.../mypy/mypy/build.py", line 256, in _build
    graph = dispatch(sources, manager, stdout)
  File "/.../mypy/mypy/build.py", line 2712, in dispatch
    process_graph(graph, manager)
  File "/.../mypy/mypy/build.py", line 3043, in process_graph
    process_stale_scc(graph, scc, manager)
  File "/.../mypy/mypy/build.py", line 3135, in process_stale_scc
    mypy.semanal_main.semantic_analysis_for_scc(graph, scc, manager.errors)
  File "/.../mypy/mypy/semanal_main.py", line 78, in semantic_analysis_for_scc
    process_top_levels(graph, scc, patches)
  File "/.../mypy/mypy/semanal_main.py", line 199, in process_top_levels
    deferred, incomplete, progress = semantic_analyze_target(next_id, state,
  File "/.../mypy/mypy/semanal_main.py", line 321, in semantic_analyze_target
    with state.wrap_context(check_blockers=False):
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/contextlib.py", line 153, in __exit__
    self.gen.throw(typ, value, traceback)
  File "/.../mypy/mypy/build.py", line 1964, in wrap_context
    yield
  File "/.../mypy/mypy/semanal_main.py", line 326, in semantic_analyze_target
    analyzer.refresh_partial(refresh_node,
  File "/.../mypy/mypy/semanal.py", line 402, in refresh_partial
    self.refresh_top_level(node)
  File "/.../mypy/mypy/semanal.py", line 411, in refresh_top_level
    self.add_implicit_module_attrs(file_node)
  File "/.../mypy/mypy/semanal.py", line 439, in add_implicit_module_attrs
    assert isinstance(node, TypeInfo)
AssertionError:
../typeshed/stdlib/collections/__init__.pyi: : note: use --pdb to drop into pdb

@KotlinIsland
Copy link
Contributor

Would proxy types help this at all?

python/typing#802

@tungol
Copy link
Contributor

tungol commented Dec 24, 2023

The error moved to mypy/checker.py - there's a lot of places in that file with the names of typing.* classes as string literals passed to TypeChecker.named_generic_type. It works if '_collections_abc.Sequence' is passed instead of 'typing.Sequence'. Using 'collections.abc.Sequence' doesn't work - it looks like 'abc' is missing from the collection module's symtable at the time it's needed.

The values in nodes.type_aliases seem fine as collections.abc and not _collections_abc, but I need to do some more testing.

A lot of noise in mypy_primer though, so something's not right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stubs: improvement Improve/refactor existing annotations, other stubs issues
Projects
None yet
Development

No branches or pull requests

7 participants