[Django] #35743: Add a method to cached_property for clearing its cached value

17 views
Skip to first unread message

Django

unread,
Sep 8, 2024, 10:14:02 AM9/8/24
to django-...@googlegroups.com
#35743: Add a method to cached_property for clearing its cached value
-------------------------------+---------------------------------------
Reporter: Jae Hyuck Sa | Type: New feature
Status: new | Component: Utilities
Version: 5.1 | Severity: Normal
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+---------------------------------------
I would like to propose an enhancement to Django's cached_property.
Currently, cached_property does not provide a built-in way to clear its
cached value from an instance. To reset the cached value, developers must
manually delete the property from the instance's __dict__.

This proposal was inspired by a suggestion made by Jacob Tyler Walls on
[this pull
request](https://github.com/django/django/pull/18534#:~:text=5%20days%20ago-,jacobtylerwalls%20reviewed%204%20days%20ago,-View%20reviewed%20changes),
where it was recommended to expose this functionality on cached_property
rather than implementing it individually for each property.

By incorporating this functionality, developers can avoid implementing
custom methods for each property, which will promote cleaner and more
maintainable code.

Feedback and suggestions from the community are greatly appreciated!
--
Ticket URL: <https://code.djangoproject.com/ticket/35743>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Sep 8, 2024, 10:14:11 AM9/8/24
to django-...@googlegroups.com
#35743: Add a method to cached_property for clearing its cached value
-------------------------------+-----------------------------------------
Reporter: Jae Hyuck Sa | Owner: Jae Hyuck Sa
Type: New feature | Status: assigned
Component: Utilities | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------------------------
Changes (by Jae Hyuck Sa ):

* owner: (none) => Jae Hyuck Sa
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/35743#comment:1>

Django

unread,
Sep 8, 2024, 10:14:27 AM9/8/24
to django-...@googlegroups.com
#35743: Add a method to cached_property for clearing its cached value
-------------------------------+-----------------------------------------
Reporter: Jae Hyuck Sa | Owner: Jae Hyuck Sa
Type: New feature | Status: assigned
Component: Utilities | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------------------------
Description changed by Jae Hyuck Sa :

Old description:

> I would like to propose an enhancement to Django's cached_property.
> Currently, cached_property does not provide a built-in way to clear its
> cached value from an instance. To reset the cached value, developers must
> manually delete the property from the instance's __dict__.
>
> This proposal was inspired by a suggestion made by Jacob Tyler Walls on
> [this pull
> request](https://github.com/django/django/pull/18534#:~:text=5%20days%20ago-,jacobtylerwalls%20reviewed%204%20days%20ago,-View%20reviewed%20changes),
> where it was recommended to expose this functionality on cached_property
> rather than implementing it individually for each property.
>
> By incorporating this functionality, developers can avoid implementing
> custom methods for each property, which will promote cleaner and more
> maintainable code.
>
> Feedback and suggestions from the community are greatly appreciated!

New description:

I would like to propose an enhancement to Django's cached_property.
Currently, cached_property does not provide a built-in way to clear its
cached value from an instance. To reset the cached value, developers must
manually delete the property from the instance's __dict__.

This proposal was inspired by a suggestion made by Jacob Tyler Walls on
this pull
request(https://github.com/django/django/pull/18534#:~:text=5%20days%20ago-,jacobtylerwalls%20reviewed%204%20days%20ago,-View%20reviewed%20changes),
where it was recommended to expose this functionality on cached_property
rather than implementing it individually for each property.

By incorporating this functionality, developers can avoid implementing
custom methods for each property, which will promote cleaner and more
maintainable code.

Feedback and suggestions from the community are greatly appreciated!

--
--
Ticket URL: <https://code.djangoproject.com/ticket/35743#comment:2>

Django

unread,
Sep 8, 2024, 11:15:43 AM9/8/24
to django-...@googlegroups.com
#35743: Add a method to cached_property for clearing its cached value
-------------------------------+-----------------------------------------
Reporter: Jae Hyuck Sa | Owner: Jae Hyuck Sa
Type: New feature | Status: assigned
Component: Utilities | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------------------------
Comment (by Simon Charette):

Given that there appears to be a consensus to move to
`functools.cached_property` once we drop support for Python 3.11 (see
#30949) I would see the addition of this method as a step that prevents us
from doing that.

What benefits are there in doing

{{{#!python
SomeClass.some_cached_property.clear_cache(some_class_instance)
}}}

over

{{{#!python
some_class_instance.__dict__.pop("some_cached_property", None)
}}}

when you can't be sure the property has been cached and `del` or `delattr`
otherwise?

It is a pattern
[https://github.com/search?q=repo%3Adjango%2Fdjango+%22__dict__.pop%22&type=code
used extensively in the code base] but that we could admittedly document
better (#30278)
--
Ticket URL: <https://code.djangoproject.com/ticket/35743#comment:3>

Django

unread,
Sep 9, 2024, 12:46:18 AM9/9/24
to django-...@googlegroups.com
#35743: Add a method to cached_property for clearing its cached value
-------------------------------+-----------------------------------------
Reporter: Jae Hyuck Sa | Owner: Jae Hyuck Sa
Type: New feature | Status: assigned
Component: Utilities | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------------------------
Comment (by Jae Hyuck Sa ):

Replying to [comment:3 Simon Charette]:
> Given that there appears to be a consensus to move to
`functools.cached_property` once we drop support for Python 3.11 (see
#30949) I would see the addition of this method as a step that prevents us
from doing that.
>
> What benefits are there in doing
>
> {{{#!python
> SomeClass.some_cached_property.clear_cache(some_class_instance)
> }}}
>
> over
>
> {{{#!python
> some_class_instance.__dict__.pop("some_cached_property", None)
> }}}
>
> when you can't be sure the property has been cached and `del` or
`delattr` otherwise?
>
> It is a pattern
[https://github.com/search?q=repo%3Adjango%2Fdjango+%22__dict__.pop%22&type=code
used extensively in the code base] but that we could admittedly document
better (#30278)

Thank you for your feedback. I was not aware of the consensus to move to
functools.cached_property once support for Python 3.11 is dropped (see
#30949). I understand that adding a method to clear the cache directly on
cached_property might hinder this transition. Additionally, I used del
because I assumed that the property would always have a cached value. I
now realize that there is a standardized pattern for clearing cached
properties, which I was not aware of before. I appreciate the feedback
and, after considering your points, I agree that better documentation is
the appropriate approach. I will raise a new issue to address this in the
documentation.

Thank you again for your valuable input.
--
Ticket URL: <https://code.djangoproject.com/ticket/35743#comment:4>

Django

unread,
Sep 9, 2024, 12:46:58 AM9/9/24
to django-...@googlegroups.com
#35743: Add a method to cached_property for clearing its cached value
-------------------------------+-----------------------------------------
Reporter: Jae Hyuck Sa | Owner: Jae Hyuck Sa
Type: New feature | Status: closed
Component: Utilities | Version: 5.1
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------------------------
Changes (by Jae Hyuck Sa ):

* resolution: => invalid
* status: assigned => closed

--
Ticket URL: <https://code.djangoproject.com/ticket/35743#comment:5>
Reply all
Reply to author
Forward
0 new messages