Skip to content

Commit

Permalink
Review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
carljm committed Oct 9, 2022
1 parent 7cb546e commit 26c2659
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 19 deletions.
7 changes: 6 additions & 1 deletion Doc/library/functools.rst
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ The :mod:`functools` module defines the following functions:
property would execute only once per instance, but it also prevents
concurrent execution on different instances of the same class, causing
unnecessary and excessive serialization of updates. This locking behavior is
deprecated and can be avoided by passing ``lock=False``.
deprecated and will be removed in Python 3.14. It can be avoided by passing
``lock=False``.

With ``lock=False``, ``cached_property`` provides no guarantee that it will
run only once per instance, if accessed in multiple threads. An application
Expand Down Expand Up @@ -127,6 +128,10 @@ The :mod:`functools` module defines the following functions:
.. versionchanged:: 3.12
The ``lock`` argument was added.

.. deprecated-removed:: 3.12 3.14
The locking behavior of ``cached_property`` is deprecated and will be
removed in 3.14. Use ``lock=False`` to avoid the deprecated behavior.


.. function:: cmp_to_key(func)

Expand Down
4 changes: 2 additions & 2 deletions Lib/functools.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
from reprlib import recursive_repr
from _thread import RLock
from types import GenericAlias
from warnings import warn


################################################################################
Expand Down Expand Up @@ -970,13 +969,14 @@ def __init__(self, func=None, *, lock=True):
if func is not None:
self.__doc__ = func.__doc__
if lock:
from warnings import warn
warn("Locking cached_property is deprecated; use @cached_property(lock=False)",
PendingDeprecationWarning, 2)
self.lock = RLock() if lock else None

def __call__(self, func=None):
if self.func is not None:
raise TypeError("'cached_property' object is not callable")
raise TypeError(f"{type(self).__name__!r} object is not callable")
self.func = func
if func is not None:
self.__doc__ = func.__doc__
Expand Down
36 changes: 20 additions & 16 deletions Lib/test/test_functools.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@
import typing
import unittest
import unittest.mock
import warnings
import weakref
import gc
from weakref import proxy
import contextlib
from inspect import Signature

from test.support.warnings_helper import ignore_warnings
from test.support import import_helper
from test.support import threading_helper

Expand Down Expand Up @@ -2931,19 +2933,21 @@ def get_cost(self):
cached_cost = py_functools.cached_property(get_cost, lock=False)


class CachedCostItemWait:
with warnings.catch_warnings():
warnings.simplefilter("ignore", category=PendingDeprecationWarning)
class CachedCostItemWait:

def __init__(self, event):
self._cost = 1
self.lock = py_functools.RLock()
self.event = event
def __init__(self, event):
self._cost = 1
self.lock = py_functools.RLock()
self.event = event

@py_functools.cached_property
def cost(self):
self.event.wait(1)
with self.lock:
self._cost += 1
return self._cost
@py_functools.cached_property
def cost(self):
self.event.wait(1)
with self.lock:
self._cost += 1
return self._cost


class CachedCostItemWaitNoLock:
Expand Down Expand Up @@ -3033,7 +3037,7 @@ def test_object_with_slots(self):

def test_immutable_dict(self):
class MyMeta(type):
@py_functools.cached_property
@py_functools.cached_property(lock=False)
def prop(self):
return True

Expand All @@ -3050,7 +3054,7 @@ def test_reuse_different_names(self):
"""Disallow this case because decorated function a would not be cached."""
with self.assertRaises(RuntimeError) as ctx:
class ReusedCachedProperty:
@py_functools.cached_property
@py_functools.cached_property(lock=False)
def a(self):
pass

Expand All @@ -3065,7 +3069,7 @@ def test_reuse_same_name(self):
"""Reusing a cached_property on different classes under the same name is OK."""
counter = 0

@py_functools.cached_property
@py_functools.cached_property(lock=False)
def _cp(_self):
nonlocal counter
counter += 1
Expand All @@ -3085,7 +3089,7 @@ class B:
self.assertEqual(a.cp, 1)

def test_set_name_not_called(self):
cp = py_functools.cached_property(lambda s: None)
cp = py_functools.cached_property(lambda s: None, lock=False)
class Foo:
pass

Expand All @@ -3098,7 +3102,7 @@ class Foo:
Foo().cp

def test_call_once_completed(self):
cp = py_functools.cached_property(lambda s: None)
cp = py_functools.cached_property(lambda s: None, lock=False)

with self.assertRaisesRegex(
TypeError,
Expand Down

0 comments on commit 26c2659

Please sign in to comment.