typing: Strengthen Type Signature of Decorators with ParamSpec. #21782
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1dea81e to
8b56bc5
Compare
|
A follow-up to this PR will be fixing the view function decorators when the mypy fix is released. |
timabbott
reviewed
Apr 13, 2022
e20ca14 to
9f81fa7
Compare
andersk
reviewed
Apr 13, 2022
andersk
reviewed
Apr 13, 2022
andersk
reviewed
Apr 13, 2022
andersk
reviewed
Apr 13, 2022
4ca2f2a to
c9b36b4
Compare
This demonstrates a way to resolve the long-standing issue of typing higher-order identity functions without using `cast` and in a type-safe manner for decorators in `cache.py`. Signed-off-by: Zixuan James Li <359101898@qq.com>
`cachify` is essentially caching the return value of a function using only the non-keyword-only arguments as the key. The use case of the function in the backend can be sufficiently covered by `functools.lru_cache` as an unbound cache. There is no signficant difference apart from `cachify` overlooking keyword-only arguments, and `functools.lru_cache` being conveniently typed. Signed-off-by: Zixuan James Li <359101898@qq.com>
Signed-off-by: Zixuan James Li <359101898@qq.com>
Signed-off-by: Zixuan James Li <359101898@qq.com>
Signed-off-by: Zixuan James Li <359101898@qq.com>
andersk
approved these changes
Apr 13, 2022
8000
timabbott
reviewed
Apr 14, 2022
| def currently_used_upload_space_bytes(self) -> int: | ||
| used_space = Attachment.objects.filter(realm=self).aggregate(Sum("size"))["size__sum"] | ||
| def currently_used_upload_space_bytes(realm) -> int: | ||
| used_space = Attachment.objects.filter(realm=realm).aggregate(Sum("size"))["size__sum"] |
There was a problem hiding this comment.
Can you confirm that the way self works as a keyword is that the current instance will still be received as the first argument even though it is not called self?
There was a problem hiding this comment.
Replying to #21782 (comment):
selfis a conventional name for the first argument of methods. It does
not affect how the instance will be passed.
I guess the Python 3 tutorial confirms that:
Often, the first argument of a method is called self. This is nothing more than a convention: the name self has absolutely no special meaning to Python. Note, however, that by not following the convention your code may be less readable to other Python programmers, and it is also conceivable that a class browser program might be written that relies upon such a convention.
I guess the crux of the issue is that a ParamSpec includes the name of positional arguments, since you can pass a positional parameter by name. OK.
There was a problem hiding this comment.
|
`self` is a conventional name for the first argument of methods. It does
not affect how the instance will be passed.
…On Thu, Apr 14, 2022, 12:24 Tim Abbott ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In zerver/models.py
<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_zulip_zulip_pull_21782-23discussion-5Fr850608581&d=DwMCaQ&c=slrrB7dE8n7gBJbeO0g-IQ&r=b_yQYQzY71v5dLCdkk1wmw&m=66e1plChRoHO3_JL6JBBRFkJKc5JPF4HA4Ie7hiIvbeUETlQryAKM1VKFC3iHh8J&s=OZzi93_lEmwTT_NQ2515N0UI3tHOCAhOhX_tIEzduNA&e=>
:
> @cache_with_key(get_realm_used_upload_space_cache_key, timeout=3600 * 24 * 7)
- def currently_used_upload_space_bytes(self) -> int:
- used_space = Attachment.objects.filter(realm=self).aggregate(Sum("size"))["size__sum"]
+ def currently_used_upload_space_bytes(realm) -> int:
+ used_space = Attachment.objects.filter(realm=realm).aggregate(Sum("size"))["size__sum"]
Can you confirm that the way self works as a keyword is that the current
instance will still be received as the first argument even though it is not
called self?
—
Reply to this email directly, view it on GitHub
<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_zulip_zulip_pull_21782-23pullrequestreview-2D942518224&d=DwMCaQ&c=slrrB7dE8n7gBJbeO0g-IQ&r=b_yQYQzY71v5dLCdkk1wmw&m=66e1plChRoHO3_JL6JBBRFkJKc5JPF4HA4Ie7hiIvbeUETlQryAKM1VKFC3iHh8J&s=RLNIeQdmQHXyWyM3GMsdVzE-uO7MEncq41ePRYEQyZE&e=>,
or unsubscribe
<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AJQG4XZDLKJVNRUMZ6CSRFDVFBBC7ANCNFSM5TJKQ3LA&d=DwMCaQ&c=slrrB7dE8n7gBJbeO0g-IQ&r=b_yQYQzY71v5dLCdkk1wmw&m=66e1plChRoHO3_JL6JBBRFkJKc5JPF4HA4Ie7hiIvbeUETlQryAKM1VKFC3iHh8J&s=vLN49rZDlkXWsAARAqECYu-F7eYQOQQBA_RVIMQknic&e=>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Merged, thanks @PIG208! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This should resolve the long-standing issue of typing higher-order identity functions without using
cast. As many decorators will be adapted to this pattern as possible.Note that before mypy releases support to
Cacatenate, we can't type certain decorators that curry or modify the parameters in any way (likehas_request_variables).Currently, we import
ParamSpecfromtyping_extensions. Hopefully, we will migrate to Python 3.10 in the future and there should be a separate PR dropping the back-port imports added here.