Skip to content

Add special support for @django.cached_property needed in django-stubs #18959

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Apr 24, 2025

Hi!

We, in django-stubs, have a lot of usages of @cached_property decorator that is a part of django: https://docs.djangoproject.com/en/5.2/ref/utils/#django.utils.functional.cached_property

All usages of it we have to add to subtest/allowlist.txt, which is not great. In typing we reuse @functools.cached_property to have all the benefits of its inference: https://github.com/typeddjango/django-stubs/blob/ee8e8b11c37866969ff0406be20591a067dfa983/django-stubs/utils/functional.pyi#L3-L4

But, stubtest is not happy with this move: because in runtime objects have django.utils.functional.cached_property type and we see the following error:


error: django.http.response.HttpResponse.text is inconsistent, cannot reconcile @property on stub with runtime object
Stub: in file /home/runner/work/django-stubs/django-stubs/django-stubs/http/response.pyi:106
def (self: django.http.response.HttpResponse) -> builtins.str
Runtime:
<django.utils.functional.cached_property object at 0x7f7ce7704920>

So, we add all @django.utils.functional.cached_property usages to our allowlist.txt. There are LOTS of entries there: https://github.com/typeddjango/django-stubs/blob/ee8e8b11c37866969ff0406be20591a067dfa983/scripts/stubtest/allowlist.txt#L158-L425

Moreover, we have to always tell about this problem to new contributors on review :(

That's why I propose to special case this as we do with other property-likes.
I've tested locally and it works perfectly. I don't want to complicate the CI with django installation and special tests. So, I added # pragma: no cover to indicate that it is not tested.

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

Successfully merging this pull request may close these issues.

4 participants