Repository navigation
Conversation
ilevkivskyi
left a comment
ilevkivskyi
left a comment
There was a problem hiding this comment.
Thanks! I left few comments.
| if isinstance(iterable, TupleType): | ||
| joined = UninhabitedType() # type: Type | ||
| for item in iterable.items: | ||
| joined = join_types(joined, item) |
There was a problem hiding this comment.
Ideally, this would be already available in the tuple fallback instance, but it doesn't work yet because of problems with forward refs (I will fix this later).
| main:20: error: "Iterable[int]" has no attribute "__aiter__"; maybe "__iter__"? | ||
| main:21: error: Iterable expected | ||
| main:21: error: "asyncify[int]" has no attribute "__iter__"; maybe "__aiter__"? | ||
| main:18: error: "Iterable[int]" has no attribute "__aiter__"; maybe "__iter__"? (not async iterable) |
There was a problem hiding this comment.
If it is easy to change, then I would skip alternative name suggestion for these particular names. If it is not easy then ignore this comment.
There was a problem hiding this comment.
Will tweak this a a bit. The __aiter__ suggestion seems potentially useful though.
| == | ||
| a.py:2: error: Argument 1 to "f" of "M" has incompatible type "int"; expected "str" | ||
|
|
||
| [case testAwaitAndAsyncDef-skip-cache] |
There was a problem hiding this comment.
Why skip cache here and in some tests below?
There was a problem hiding this comment.
To avoid running the test cases twice -- the second run doesn't seem very useful.
| main:5: error: Incompatible return value type (got "str", expected "int") | ||
| == | ||
| == | ||
| main:4: error: "C" has no attribute "__aexit__" |
There was a problem hiding this comment.
I would add few tests where iterators and context managers change kind (i.e. from normal to async and back).
There was a problem hiding this comment.
I wrote these tests and they passed but they don't seem to increase test coverage in a meaningful way so I'd like to leave them out. (Not sure if you've noticed but I'm getting tired of the slow local test runs... I suspect that the tests are over 2x slower than not too long ago.)
There was a problem hiding this comment.
I didn't notice they are so much slower. Last run took 155 sec. on my laptop, which is not much. Maybe we can think about making tests more lightweight, thinking about which tests to include may impose more mental burden than actually waiting, but this may be subjective.
| from a import C | ||
|
|
||
| async def f() -> int: | ||
| async with C() as x: |
There was a problem hiding this comment.
I would add a test without assignment target (x in this case).
There was a problem hiding this comment.
Will add to this same test.
ilevkivskyi
left a comment
ilevkivskyi
left a comment
There was a problem hiding this comment.
Note there are merge conflicts now, otherwise looks good.
Some of the changes motivated cleaning up some duplicate error
messages.
Also noticed missing
__next__/nextdependency in ordinary forstatements and fixed it as well.
Work towards #4951.