Add special support for @django.cached_property needed in django-stubs#18959
Add special support for @django.cached_property needed in django-stubs#18959
@django.cached_property needed in django-stubs#18959Conversation
mypy/stubtest.py
Outdated
| # https://docs.djangoproject.com/en/5.2/ref/utils/#django.utils.functional.cached_property | ||
| # This is needed in `django-stubs` project: | ||
| # https://github.com/typeddjango/django-stubs | ||
| if repr(type(runtime)) != "<class 'django.utils.functional.cached_property'>": |
There was a problem hiding this comment.
What do you think of using type(runtime).__name__ == "cached_property" as a heuristic? That way we're not as specific to one user.
Also out of curiosity, why can't django use @functools.cached_property directly?
There was a problem hiding this comment.
Also out of curiosity, why can't django use @functools.cached_property directly?
No idea 🤷
What do you think of using type(runtime).name == "cached_property" as a heuristic? That way we're not as specific to one user.
I like your idea better, because it does not call __repr__, which might have side-effects. Of course, .__name__ might also trigger some very complex cases, but they should be extrimelly rare.
There was a problem hiding this comment.
Why can't django use
@functools.cached_propertydirectly?
Django predates python 3.8 by many years, and probably there's little to no benefit from removing their in-house version. A lot (really a lot) of users import cached_property from Django, so they'll have to maintain an alias anyway.
There was a problem hiding this comment.
Yup. Django's version long predates Python's. Also, when it was originally introduced in Python 3.8, the implementation had a lock that caused a lot of problems which was only only removed in Python 3.12. As Django 6.0 will target Python 3.12+ we can probably look at deprecating Django's version, or at least making it an alias of the standard library version.
|
I will give @JukkaL some time to re-review and will merge it in several days. |
|
Thanks everyone! This will really help for our use-case :) |
6 participants
Hi!
We, in
django-stubs, have a lot of usages of@cached_propertydecorator that is a part ofdjango: https://docs.djangoproject.com/en/5.2/ref/utils/#django.utils.functional.cached_propertyAll usages of it we have to add to
subtest/allowlist.txt, which is not great. In typing we reuse@functools.cached_propertyto have all the benefits of its inference: https://github.com/typeddjango/django-stubs/blob/ee8e8b11c37866969ff0406be20591a067dfa983/django-stubs/utils/functional.pyi#L3-L4But,
stubtestis not happy with this move: because in runtime objects havedjango.utils.functional.cached_propertytype and we see the following error:So, we add all
@django.utils.functional.cached_propertyusages to ourallowlist.txt. There are LOTS of entries there: https://github.com/typeddjango/django-stubs/blob/ee8e8b11c37866969ff0406be20591a067dfa983/scripts/stubtest/allowlist.txt#L158-L425Moreover, 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
djangoinstallation and special tests. So, I added# pragma: no coverto indicate that it is not tested.