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

Give execute and maybeDeferred correct type annotations #11696

Closed
exarkun opened this issue Sep 30, 2022 · 3 comments · Fixed by #11697
Closed

Give execute and maybeDeferred correct type annotations #11696

exarkun opened this issue Sep 30, 2022 · 3 comments · Fixed by #11697

Comments

@exarkun
Copy link
Member

exarkun commented Sep 30, 2022

Currently maybeDeferred is annotated:

def maybeDeferred(
    f: Callable[..., _T], *args: object, **kwargs: object
) -> "Deferred[_T]":

The handling of the parameters of f and of args and kwargs is overly general. The handling of _T is incomplete and incorrect with respect to Deferred and coroutine.

execute is in roughly the same situation.

@arnimarj
Copy link

For what it's worth, I've been using this for a while now:

_P = ParamSpec('_P')
...
def maybeDeferred(f: Union[Callable[_P, _T], Awaitable[_T]], *args: _P.args, **kwargs: _P.kwargs) -> Deferred[_T]:
	...

@exarkun
Copy link
Member Author

exarkun commented Sep 30, 2022

Thanks. I think the Union there is slightly wrong in a couple ways:

  • f is not an Awaitable itself but a function that returns a value, possibly in some kind of async container.
  • If the Union moves inside like Callable[_P, Union[Awaitable[_T], _T]] then the return type expresses nothing more than Callable[_P, _T] because _T includes all of Awaitable[_T] (ie, _T might be Awaitable[_S]).

So unfortunately the best type I can figure out for f is Callable[_P, _T] even though this clearly fails to capture a lot of interesting stuff about the type.

@exarkun exarkun changed the title Give maybeDeferred correct type annotations Give execute and maybeDeferred correct type annotations Sep 30, 2022
@yogiwahyuuu

This comment was marked as off-topic.

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 a pull request may close this issue.

3 participants