Make collections.abcs more consistent with runtime implementation #10816

Gobot1234 · 2023-10-01T18:59:12Z

Interested to see how much breakage primer reports.

Gobot1234 · 2023-10-01T23:58:42Z

Looking over these issues, all the aiohttp ones look like a mypy bug where it thinks that any annotated attribute is abstract the others hopefully might be easy enough fixes if you rename typing.Generator strings to types.GeneratorType, I'll see if a mypy PR can get rid of most of them. If that isn't possible would it be possible for mypy to selectively remove this commit?

srittau · 2023-10-02T09:05:59Z

stdlib/types.pyi

@@ -358,6 +358,12 @@ _ReturnT_co = TypeVar("_ReturnT_co", covariant=True)

 @final
 class GeneratorType(Generator[_YieldT_co, _SendT_contra, _ReturnT_co]):


I'd be interested what happens if we just change types.pyi, but remove the typing base classes to better match runtime.

I'm not entirely sure what you're proposing, wouldn't that just crash mypy because typing.Generator wouldn't exist?

I meant just using class GeneratorType: here (and adding the methods), not deleting typing.Generator.

typing.Generator hasn't been deleted it's just a protocol now

Gobot1234 · 2023-10-02T09:16:17Z

stdlib/typing.pyi

@@ -419,32 +419,16 @@ class Generator(Iterator[_YieldT_co], Generic[_YieldT_co, _SendT_contra, _Return
    @abstractmethod
    def throw(self, __typ: BaseException, __val: None = None, __tb: TracebackType | None = None) -> _YieldT_co: ...
    def close(self) -> None: ...
-    def __iter__(self) -> Generator[_YieldT_co, _SendT_contra, _ReturnT_co]: ...


I'm unsure as to whether or not this should be -> Self the runtime has it as that in Iterator

Upon further consideration, not implementing it as returning Self is probably a bug so I'll change it in (Async)Iterator even

typing.Generator.__iter__ shouldn't be annotated as returning Self for the same reason that typing.Iterator.__iter__ shouldn't be annotated as returning Self. 99% of all iterators have __iter__ methods that return Self, and 99% of generators are the same. But if we hinted them as such in the stub, we'd be changing the very definition of Iterator/Generator such that the remaining 1% of iterators and generators would no longer be considered iterators or generators. And that wouldn't be accurate: it's not part of the contract that iterators have to return self from their __iter__ methods at runtime in order for them to be considered iterators, they just have to return an iterable object.

With types.GeneratorType, it's a different issue, because that class doesn't define an interface in the same way. We can just reflect what happens at runtime there.

I suspect removing this method is causing the mypy-primer fallout. Let's add it back.

Gobot1234 · 2023-10-02T09:17:42Z

stdlib/types.pyi

@@ -358,6 +358,12 @@ _ReturnT_co = TypeVar("_ReturnT_co", covariant=True)

 @final
 class GeneratorType(Generator[_YieldT_co, _SendT_contra, _ReturnT_co]):


I'm not entirely sure what you're proposing, wouldn't that just crash mypy because typing.Generator wouldn't exist?

JelleZijlstra · 2024-10-02T01:08:25Z

This change feels correct, let's see if we can get type checkers to agree.

stdlib/typing.pyi

JelleZijlstra · 2024-10-17T03:39:13Z

cc @A5rocks regarding the primer changes in trio. I think they are technically correct; trio uses the Coroutine ABC, which does not guarantee the existence of attributes like cr_frame. However, I'm not sure this makes the user experience better in practice.

A5rocks · 2024-10-17T04:10:05Z

cc @A5rocks regarding the primer changes in trio. I think they are technically correct; trio uses the Coroutine ABC, which does not guarantee the existence of attributes like cr_frame. However, I'm not sure this makes the user experience better in practice.

I agree with these changes being technically correct, because asyncio.iscoroutine exists. (I'm not too sure about if it didn't exist, but oh well. I see something about a types.coroutine decorator but I can't get a collections.abc.Coroutine isinstance check to pass.)

>>> import asyncio
>>> @asyncio.coroutine
... def blah():
...   yield "hello"
...
>>> asyncio.iscoroutine(blah())
True
>>> blah().cr_frame
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'generator' object has no attribute 'cr_frame'

Anyways, I think it's better to be more correct. We have added asserts to check cr_frame in the past (because it's Optional) and this is just more along that line. Though take what I say with a grain of salt because I don't really know the internal details.

…ons.abc

github-actions · 2024-12-28T05:23:49Z

Diff from mypy_primer, showing the effect of this PR on open source code:

trio (https://github.com/python-trio/trio)
+ src/trio/_core/_traps.py:307: error: "Coroutine[Any, Outcome[object], Any]" has no attribute "cr_running"  [attr-defined]
+ src/trio/_core/_ki.py:179: error: "Coroutine[Any, Outcome[object], Any]" has no attribute "cr_frame"  [attr-defined]
+ src/trio/_core/_run.py:1882: error: "Coroutine[object, Never, object]" has no attribute "cr_frame"  [attr-defined]
+ src/trio/_core/_tests/test_asyncgen.py:91: error: "AsyncGenerator[int, None]" has no attribute "ag_frame"  [attr-defined]
+ src/trio/_core/_tests/test_asyncgen.py:304: error: "AsyncGenerator[object, Never]" has no attribute "ag_frame"  [attr-defined]
+ src/trio/_core/_tests/test_asyncgen.py:307: error: "AsyncGenerator[object, Never]" has no attribute "ag_frame"  [attr-defined]

Gobot1234 and others added 2 commits October 1, 2023 19:58

Make more consistent with runtime implementation

78a77de

[pre-commit.ci] auto fixes from pre-commit.com hooks

0fd644f

Gobot1234 changed the title ~~Make more consistent with runtime implementation~~ Make collections.abcs more consistent with runtime implementation Oct 1, 2023