Skip to content

bpo-21145: Add cached_property decorator in functools #6982

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.

Sign up for GitHub

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

Merged
ncoghlan merged 12 commits into from
Aug 28, 2018
Merged

bpo-21145: Add cached_property decorator in functools #6982

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1cda951
bpo-21145: Add cached_property decorator in functools
carljm May 19, 2018
72259cd
Review feedback.
carljm May 20, 2018
26f3f43
More review feedback.
carljm May 20, 2018
567e819
More review feedback.
carljm May 22, 2018
f7876d0
Minor review comments.
carljm Jul 28, 2018
354a975
Allow __set_name__ to set cached attribute name.
carljm Jul 28, 2018
f006f68
Fix consistency of doc example.
carljm Jul 28, 2018
d6bed4b
Explicitly require calling __set_name__ exactly once on cached_property.
carljm Jul 29, 2018
b41380d
Refine error message.
carljm Jul 29, 2018
b504a20
Allow reuse of cached_property as long as name remains the same.
carljm Jul 30, 2018
977ece0
Merge branch 'master' into cached-property
carljm Aug 19, 2018
5bdeaab
Clearer error message when __dict__ is not mutable.
carljm Aug 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Allow __set_name__ to set cached attribute name.
  • Loading branch information
@carljm
carljm committed Jul 28, 2018
commit 354a975207e3c8c279a0e7311375937654802858
13 changes: 8 additions & 5 deletions Lib/functools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -880,26 +880,29 @@ def __isabstractmethod__(self):
class cached_property:
def __init__(self, func):
self.func = func
self.attrname = func.__name__

@sir-sigurd sir-sigurd Jul 29, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks redundant since attrname is set in __set_name__()

@carljm carljm Jul 29, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you suggest to do instead? This is the most reasonable fallback in case __set_name__ is not called in some unusual use of this class, and it avoids the object ever being in an incompletely initialized state.

self.__doc__ = func.__doc__
self.lock = RLock()

def __set_name__(self, owner, name):
self.attrname = name

@sir-sigurd sir-sigurd Jul 29, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might make sense to raise error in this case, since a will never be cached:

class C:
    @cached_property
    def a(self):
        pass
    b = a

@carljm carljm Jul 29, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I guess that also answers the above question; we will raise error in case __set_name__ is ever called twice, and also I guess raise error if __get__ is called and __set_name__ was not.


def __get__(self, instance, owner):
if instance is None:
return self
attrname = self.func.__name__
try:
cache = instance.__dict__
except AttributeError: # objects with __slots__ have no __dict__

@ncoghlan ncoghlan Aug 4, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tweak the comment here along similar lines to the note in the docs

msg = (
f"No '__dict__' attribute on {type(instance).__name__!r} "
f"instance to cache {attrname!r} property."
f"instance to cache {self.attrname!r} property."
)
raise TypeError(msg) from None
val = cache.get(attrname, _NOT_FOUND)
val = cache.get(self.attrname, _NOT_FOUND)
if val is _NOT_FOUND:
with self.lock:
# check if another thread filled cache while we awaited lock
val = cache.get(attrname, _NOT_FOUND)
val = cache.get(self.attrname, _NOT_FOUND)
if val is _NOT_FOUND:
val = cache[attrname] = self.func(instance)
val = cache[self.attrname] = self.func(instance)

@ncoghlan ncoghlan Aug 4, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the AttributeError above, it's likely worth wrapping TypeError here to account for cases like:

>>> str.__dict__["x"] = 1
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'mappingproxy' object does not support item assignment

return val
19 changes: 19 additions & 0 deletions Lib/test/test_functools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2313,6 +2313,7 @@ def f(*args):
with self.assertRaisesRegex(TypeError, msg):
f()


class CachedCostItem:
_cost = 1

Expand All @@ -2327,6 +2328,17 @@ def cost(self):
return self._cost


class OptionallyCachedCostItem:
_cost = 1

@sir-sigurd sir-sigurd Jun 17, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed? It seems to be always shadowed in __init__().


def get_cost(self):
"""The cost of the item."""
self._cost += 1
return self._cost

cached_cost = py_functools.cached_property(get_cost)


class CachedCostItemWait:

def __init__(self, event):
Expand Down Expand Up @@ -2359,6 +2371,13 @@ def test_cached(self):
self.assertEqual(item.cost, 2)
self.assertEqual(item.cost, 2) # not 3

def test_cached_attribute_name_differs_from_func_name(self):
item = OptionallyCachedCostItem()
self.assertEqual(item.get_cost(), 2)
self.assertEqual(item.cached_cost, 3)
self.assertEqual(item.get_cost(), 4)
self.assertEqual(item.cached_cost, 3)

def test_threaded(self):
go = threading.Event()
item = CachedCostItemWait(go)
Expand Down