Skip to content

Improve gather typing when coros more than 5 #9105

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

Closed
wants to merge 1 commit into from
Closed

Improve gather typing when coros more than 5 #9105

wants to merge 1 commit into from

Conversation

mnixry
Copy link
Contributor

@mnixry mnixry commented Nov 5, 2022

No description provided.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2022

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

core (https://github.com/home-assistant/core)
+ homeassistant/requirements.py:214: error: Subclass of "Integration", "BaseException", and "IntegrationNotFound" cannot exist: would have inconsistent method resolution order  [unreachable]
+ homeassistant/requirements.py:214: error: Right operand of "or" is never evaluated  [unreachable]
+ homeassistant/helpers/trigger.py:198: error: Statement is unreachable  [unreachable]
+ homeassistant/helpers/trigger.py:200: error: Statement is unreachable  [unreachable]
+ homeassistant/helpers/trigger.py:202: error: Statement is unreachable  [unreachable]
+ homeassistant/components/amcrest/camera.py:411: error: Incompatible types in assignment (expression has type "object", variable has type "bool")  [assignment]
+ homeassistant/components/amcrest/camera.py:411: error: Incompatible types in assignment (expression has type "object", variable has type "Optional[bool]")  [assignment]
+ homeassistant/components/amcrest/camera.py:411: error: Incompatible types in assignment (expression has type "object", variable has type "Optional[str]")  [assignment]

@JelleZijlstra
Copy link
Member

This probably makes things worse in practice in cases where the gathered futures are heterogeneous. _T will be inferred to be either a join like object (mypy) or a big union (pyright), and you'll get false-positive errors because that type is not practical to work with.

@mnixry
Copy link
Contributor Author

mnixry commented Nov 6, 2022

This probably makes things worse in practice in cases where the gathered futures are heterogeneous. _T will be inferred to be either a join like object (mypy) or a big union (pyright), and you'll get false-positive errors because that type is not practical to work with.

Thank you for your reply. I have experienced the type become an object when I'm using MyPy. Since I have used Pyright for work, I almost forgot about this. I just don't like to write “Anython” for the gathered result :)

Could you please give me some advice to improve this PR?

@JelleZijlstra
Copy link
Member

I'm afraid there may not be a way to improve this type in the current type system. (Well, we could add overloads for more than 5 futures, but where to stop?)

PEP 646's variadic TypeVars seem like they would help, but it's not enough because we need to go from _FutureLike[T] to T for each element, and PEP 646 doesn't let you do that. There's been talk of a type-level Map operator, but there is no PEP for that yet. If that existed, we'd be able to write something like def gather(*futures: *Map[_FutureLike, Ts]) -> tuple[*Ts]: ....

Though we'd still have the problem that gather() returns a list, not a tuple. But there is no way to type a heterogeneous list, and there hasn't been a serious proposal to do that. So pragmatically, we may have to continue to lie and claim gather() returns a tuple.

Another alternative could be to write special-case logic in a type checker. Mypy may be receptive to that; pyright probably not.

@mnixry
Copy link
Contributor Author

mnixry commented Nov 7, 2022

Thank you for your explanation. It seems there are no very great solution util the Map operator becomes available :(

I will close this PR soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants