[channels] Make URLRouter.routes a Collection#15080
[channels] Make URLRouter.routes a Collection#15080cjwatson wants to merge 2 commits intopython:mainfrom
Conversation
This comment has been minimized.
This comment has been minimized.
|
I think it makes sense to migrate from list to Collection. If you have time, could you also help check whether other places or functionalities could be updated to use Collection as well? |
`list` is invariant, which made this type inconvenient in practice. Each of the routes is either a pattern or another router.
ff63966 to
89dcb62
Compare
|
Good point - this is only iterated over, not indexed, so I only found a couple of other places in |
9a80b21 to
79675e2
Compare
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
|
Looks good to me now. |
|
To make it easier for the core developers to track and approve this PR, could you create an issue and link this PR to it, @cjwatson? |
|
And I think you can make the title a bit more generic since the changes are varied. |
| routes: Collection[_ExtendedURLPattern | URLRouter] | ||
|
|
||
| def __init__(self, routes: list[_ExtendedURLPattern | URLRouter]) -> None: ... | ||
| def __init__(self, routes: Collection[_ExtendedURLPattern | URLRouter]) -> None: ... |
There was a problem hiding this comment.
The typeshed standard for for ... in loops is Iterable. In fact, Collection won't work:
from collections.abc import Container
x: Container[int] = []
for y in x: # error: "Container[int]" has no attribute "__iter__" (not iterable)
reveal_type(y) # note: Revealed type is "Any"| async def await_many_dispatch( | ||
| consumer_callables: list[Callable[[], Awaitable[ASGIReceiveCallable]]], dispatch: Callable[[dict[str, Any]], Awaitable[None]] | ||
| consumer_callables: Sequence[Callable[[], Awaitable[ASGIReceiveCallable]]], | ||
| dispatch: Callable[[dict[str, Any]], Awaitable[None]], |
There was a problem hiding this comment.
Since the return value of dispatch is ignored, it can be an arbitrary value and doesn't have to be None:
| dispatch: Callable[[dict[str, Any]], Awaitable[None]], | |
| dispatch: Callable[[dict[str, Any]], Awaitable[object]], |
| @@ -1,13 +1,19 @@ | |||
| from collections.abc import Collection | |||
For reference, we try to cut back on the use of pseudo-protocols like |
listis invariant, which made this type inconvenient in practice. Each of the routes is either a pattern or another router.