-
-
Notifications
You must be signed in to change notification settings - Fork 868
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
Fix authentication.requires
type
#1659
Conversation
I like the enthusiasm! Thanks for the PR @sobolevn ! 🙇 But... This is not the right annotation, that's why the For those reasons, I'll be closing this PR. |
It looks right to me 🤔 This is the common way of expressing "this function returns a decorator that preserves the signature".
Sure thing, I will open an issue about that. |
@Kludex I do think this is the right annotation, unless we want to be more explicit and encode |
starlette/authentication.py
Outdated
@@ -95,7 +97,7 @@ def sync_wrapper(*args: typing.Any, **kwargs: typing.Any) -> Response: | |||
|
|||
return sync_wrapper | |||
|
|||
return decorator | |||
return decorator # type: ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add at least the error on ignore[<error>]
? 🙏
starlette/authentication.py
Outdated
@@ -95,7 +97,7 @@ def sync_wrapper(*args: typing.Any, **kwargs: typing.Any) -> Response: | |||
|
|||
return sync_wrapper | |||
|
|||
return decorator | |||
return decorator # type: ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return decorator # type: ignore | |
return decorator # type: ignore[return-value] # type checkers can't verify the type of the wrapper |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm already happy with just the return-value thingy, no need the comment stuff
Thanks for the explanation! I guess I got excited closing PRs that day. 😰 I retract my words from before, and sorry for closing the PR. :) |
Thank you for the contribution @sobolevn ! |
I've noticed that
authentication.requires
does not preserve type information about the passed function.So, I've fixed that!