Skip to content

Make router decorators gentle towards types#1470

Closed
antonagestam wants to merge 2 commits intoKludex:masterfrom
antonagestam:fix/dont-squash-exception-handler-typing
Closed

Make router decorators gentle towards types#1470
antonagestam wants to merge 2 commits intoKludex:masterfrom
antonagestam:fix/dont-squash-exception-handler-typing

Conversation

@antonagestam
Copy link

What does this do? These decorators currently destroy the signatures of the functions they're applied to. We remedy this by using a typevar, which makes the signature of the decorators say the equivalent of "I take any callable, and I will return the exact type that I'm given".

With this example:

from starlette.applications import Starlette
from starlette.requests import Request
from starlette.responses import Response

app = Starlette()


@app.route("/")
def handle_request(request: Request) -> Response:
    return Response()

reveal_type(handle_request)

The current behavior is:

demo.py:12: note: Revealed type is "Any"

But with this PR applied:

demo.py:12: note: Revealed type is "def (request: starlette.requests.Request) -> starlette.responses.Response"

@adriangb
Copy link
Contributor

adriangb commented Feb 1, 2022

One the one hand I think this is a pretty reasonable typing improvement.
On the other hand:

 # The following usages are now discouraged in favour of configuration
 #  during Starlette.__init__(...)

@adriangb adriangb added the typing Type annotations or mypy issues label Feb 2, 2022
@Kludex
Copy link
Owner

Kludex commented Apr 24, 2022

I think we should be strong on decisions: if we want to remove it at some point, let's deprecate it first...

I really want this, but if we don't want to estimulate the usage of those decorators, to me it doesn't make sense to make it "more correct" type wise.

In any case, I'll be closing this PR just because of the in-code comment we have, and because I'm trying to reduce the amount of stale PRs we have.

Thanks for the PR @antonagestam , and sorry for taking long to take a decision here!

@Kludex Kludex closed this Apr 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

typing Type annotations or mypy issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants