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

incorrect type annotation on inlineCallbacks #10231

Closed
twisted-trac opened this issue Jul 16, 2021 · 7 comments
Closed

incorrect type annotation on inlineCallbacks #10231

twisted-trac opened this issue Jul 16, 2021 · 7 comments

Comments

@twisted-trac
Copy link

richvdh's avatar @richvdh reported
Trac ID trac#10231
Type release blocker: regression
Created 2021-07-16 09:12:43Z

defer.inlineCallbacks currently has the following signature:

def inlineCallbacks(
    f: Callable[..., Generator[Deferred[object], object, None]]
) -> Callable[..., Deferred[object]]:

Three things here:

  • In the result of f, why do the YieldType and SendType use object rather than Any? There is nothing to say that the Deferreds yielded will resolve to an object rather than, say, a float.
  • Similarly, why is None the result of the Generator rather than Any (or a type variable)? Functions wrapped by inlineCallbacks have been allowed to return things ever since Python 3.3 added support for return in generators.
  • Finally: why do we claim that the returned callable will return a Deferred[object]? Again, there is nothing to say that it will be an object rather than a float.

I think it should be:

def inlineCallbacks(
    f: Callable[..., Generator[Deferred[Any], Any, _T]]
) -> Callable[..., Deferred[_T]]:
Searchable metadata
trac-id__10231 10231
type__release_blocker__regression release blocker: regression
reporter__richvdh richvdh
priority__normal normal
milestone__None None
branch__ 
branch_author__ 
status__closed closed
resolution__fixed fixed
component__core core
keywords__None None
time__1626426763106885 1626426763106885
changetime__1626945958588291 1626945958588291
version__None None
owner__graingert graingert
cc__graingert
@twisted-trac
Copy link
Author

graingert's avatar @graingert commented

why do we claim that the returned callable will return a Deferred[object]? Again, there is nothing to say that it will be an object rather than a float.

a float is an object:

>>> float.__mro__
(<class 'float'>, <class 'object'>)

Similarly, why is None the result of the Generator rather than Any (or a type variable)? Functions wrapped by inlineCallbacks have been allowed to return things ever since Python 3.3 added support for return in generators.

Generator[Deferred[object], object, object]] I think is correct. The problem is Generator functions don't pass the type from their yield to their sent item:

@inlineCallbacks
def foo():
    v = Deferred[int]()
    u = Deferred[str]()

    protocol(u, v)
    x = yield v
    y = yield u
    reveal_type(x)  # afaik there's no way to set the type of x or y here from u or v
    reveal_type(y)

@twisted-trac
Copy link
Author

graingert's avatar @graingert commented
P = ParamSpec("P")

def inlineCallbacks(f: Callable[P, Generator[Deferred[T], T, R]]) -> Callable[P, Deferred[R]]:

I think is closer - but this only allows the same type of Deferred to be yielded

@twisted-trac
Copy link
Author

graingert's avatar @graingert commented

the following can work though:

@inlineCallbacks
def foo():
    v = Deferred[int]()
    u = Deferred[str]()

    protocol(u, v)
    x = yield from v
    y = yield from u
    reveal_type(x)  # int
    reveal_type(y)  # str

and that's fixed here: https://github.com/twisted/twisted/pull/1624/files#r668723516

@twisted-trac
Copy link
Author

richvdh's avatar @richvdh commented

Replying to Thomas Grainger:

why do we claim that the returned callable will return a Deferred[object]? Again, there is nothing to say that it will be an object rather than a float.

a float is an object:

hrm. Today I learnt!

@twisted-trac
Copy link
Author

richvdh's avatar @richvdh commented

I've put up a PR to update the result type using a TypeVar, at #1632.

@twisted-trac
Copy link
Author

glyph's avatar @glyph set owner to @graingert

Please try to keep ticket state up to date

@twisted-trac
Copy link
Author

richvdh's avatar @richvdh set status to closed

Thomas: I'm going to mark this issue resolved, as from my point of view 21.7.0rc2 is very much good enough for now. Hope that's ok. You're right that it's not perfect but it's not obvious to me we can really do better without significant extra complexity.

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

No branches or pull requests

2 participants