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

Deferred's type annotations are generally wrong #11772

Closed
glyph opened this issue Nov 22, 2022 · 2 comments · Fixed by #11770
Closed

Deferred's type annotations are generally wrong #11772

glyph opened this issue Nov 22, 2022 · 2 comments · Fixed by #11770
Labels

Comments

@glyph
Copy link
Member

glyph commented Nov 22, 2022

The current type annotations have a number of issues:

  • _DeferredResultT is contravariant, which is confusing and inaccurate; it should be invariant
  • addCallback, addErrback, and addBoth don't check their callbacks' signatures at all; you don't have to accept a Failure for the latter two, to say nothing of the rest of the signature.
  • Deferred.fromFuture uses an unspecialized generic (see Improve type annotation for twisted.internet.defer.Deferred.fromFuture #11753)
  • maybeDeferred incorrectly returns a Deferred[Deferred[X]] if its function returns a deferred

Some of these are due to new features in Mypy that have come out since the type hints were added, some are simply errors in the initial implementation, but we should give it a new pass to address both and have more correct stubs. (Some issues are not possible to fix, such as addCallabcks having multiple ParamSpec signatures in a way that mypy does not yet allow expressing.)

@DMRobertson
Copy link
Contributor

Possibly of interest: I mooted in matrix-org/synapse#15008 (comment) that Deferred should perhaps be generic over two types: the initial value fed to the first callback, and the return value of the last callback.

@glyph
Copy link
Member Author

glyph commented Mar 16, 2023

@DMRobertson I want to split Deferred into two protocols for that, actually, and have a thing that returns Callbackable[...] and AddCallbackable[...]. But that's a big lift :-|

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

Successfully merging a pull request may close this issue.

2 participants