Skip to content

Make router decorators gentle towards types #1470

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Make router decorators gentle towards types #1470

wants to merge 2 commits into from

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"

@antonagestam antonagestam mentioned this pull request Feb 1, 2022
Closed
@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
Collaborator

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
Reviewers
No reviews
Assignees
No one assigned
Labels
typing Type annotations or mypy issues
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@antonagestam @adriangb @Kludex