From da3ec333a4a8847cb1bf05fd8ea80beeaeb64a95 Mon Sep 17 00:00:00 2001 From: Matt Wilber Date: Mon, 15 Oct 2018 23:44:35 +0000 Subject: [PATCH 1/6] bpo-34995: Maintain func.__isabstractmethod__ in functools.cached_property --- Lib/functools.py | 1 + Lib/test/test_functools.py | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/Lib/functools.py b/Lib/functools.py index 51048f5946c346..8ee115f22be87d 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -882,6 +882,7 @@ def __init__(self, func): self.func = func self.attrname = None self.__doc__ = func.__doc__ + self.__isabstractmethod__ = getattr(func, '__isabstractmethod__', False) self.lock = RLock() def __set_name__(self, owner, name): diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index 200a5eb4955c1d..f727cd34957792 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -2478,6 +2478,16 @@ def test_access_from_class(self): def test_doc(self): self.assertEqual(CachedCostItem.cost.__doc__, "The cost of the item.") + def test_isabstractmethod(self): + class AbstractExpensiveCalculator(abc.ABC): + @functools.cached_property + @abc.abstractmethod + def calculate(self): + pass + + with self.assertRaises(TypeError): + AbstractExpensiveCalculator() + if __name__ == '__main__': unittest.main() From bfdc2499a620b8688f136d30c5c4dca672a489d0 Mon Sep 17 00:00:00 2001 From: Matt Wilber Date: Mon, 15 Oct 2018 23:57:25 +0000 Subject: [PATCH 2/6] Add news entry --- .../NEWS.d/next/Library/2018-10-15-23-56-39.bpo-34995.kNA9ge.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2018-10-15-23-56-39.bpo-34995.kNA9ge.rst diff --git a/Misc/NEWS.d/next/Library/2018-10-15-23-56-39.bpo-34995.kNA9ge.rst b/Misc/NEWS.d/next/Library/2018-10-15-23-56-39.bpo-34995.kNA9ge.rst new file mode 100644 index 00000000000000..22262c4953c117 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-10-15-23-56-39.bpo-34995.kNA9ge.rst @@ -0,0 +1 @@ +Maintain wrapped method's __isabstractmethod__ in functools.cached_property From 16c0372c8e0b67f55f5a7796f9ac80b14e23fa1e Mon Sep 17 00:00:00 2001 From: Matt Wilber Date: Thu, 25 Oct 2018 00:23:50 +0000 Subject: [PATCH 3/6] Only set __isabstractmethod__ if wrapapped func sets it --- Lib/functools.py | 4 +++- Lib/test/test_functools.py | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Lib/functools.py b/Lib/functools.py index 8ee115f22be87d..23d14a77120126 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -882,7 +882,9 @@ def __init__(self, func): self.func = func self.attrname = None self.__doc__ = func.__doc__ - self.__isabstractmethod__ = getattr(func, '__isabstractmethod__', False) + func_isabstractmethod = getattr(func, '__isabstractmethod__') + if func_isabstractmethod is not None: + self.__isabstractmethod__ = func_isabstractmethod self.lock = RLock() def __set_name__(self, owner, name): diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index f727cd34957792..cb09c3a10922e7 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -2485,6 +2485,7 @@ class AbstractExpensiveCalculator(abc.ABC): def calculate(self): pass + self.assertTrue(getattr(AbstractExpensiveCalculator.calculate, '__isabstractmethod__', False)) with self.assertRaises(TypeError): AbstractExpensiveCalculator() From db8dcd7fb48b84ea3c548ac07422846d9157ab02 Mon Sep 17 00:00:00 2001 From: Matt Wilber Date: Thu, 25 Oct 2018 00:33:49 +0000 Subject: [PATCH 4/6] Check for __isabstractmethod__ with try/except --- Lib/functools.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Lib/functools.py b/Lib/functools.py index 23d14a77120126..cb8845b48a3ffc 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -882,9 +882,10 @@ def __init__(self, func): self.func = func self.attrname = None self.__doc__ = func.__doc__ - func_isabstractmethod = getattr(func, '__isabstractmethod__') - if func_isabstractmethod is not None: - self.__isabstractmethod__ = func_isabstractmethod + try: + self.__isabstractmethod__ = func.__isabstractmethod__ + except AttributeError: + pass self.lock = RLock() def __set_name__(self, owner, name): From 1c91dd20d759b0e6a7cec42919cdc2cf285de0fe Mon Sep 17 00:00:00 2001 From: Matt Wilber Date: Thu, 25 Oct 2018 18:33:15 +0000 Subject: [PATCH 5/6] Add negative test case for abstract cached_property --- Lib/test/test_functools.py | 13 +++++++++++-- .../2018-10-15-23-56-39.bpo-34995.kNA9ge.rst | 2 +- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index cb09c3a10922e7..46bd7cab93e78e 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -2478,17 +2478,26 @@ def test_access_from_class(self): def test_doc(self): self.assertEqual(CachedCostItem.cost.__doc__, "The cost of the item.") - def test_isabstractmethod(self): + def test_isabstractmethod_copied(self): class AbstractExpensiveCalculator(abc.ABC): @functools.cached_property @abc.abstractmethod def calculate(self): pass - self.assertTrue(getattr(AbstractExpensiveCalculator.calculate, '__isabstractmethod__', False)) + self.assertTrue(AbstractExpensiveCalculator.calculate.__isabstractmethod__) with self.assertRaises(TypeError): AbstractExpensiveCalculator() + def test_missing_isabstractmethod_not_copied(self): + class ConcreteExpensiveCalculator(abc.ABC): + @functools.cached_property + def calculate(self): + return 42 + + self.assertFalse(getattr(ConcreteExpensiveCalculator.calculate, '__isabstractmethod__', False)) + ConcreteExpensiveCalculator() + if __name__ == '__main__': unittest.main() diff --git a/Misc/NEWS.d/next/Library/2018-10-15-23-56-39.bpo-34995.kNA9ge.rst b/Misc/NEWS.d/next/Library/2018-10-15-23-56-39.bpo-34995.kNA9ge.rst index 22262c4953c117..bc33287c180f18 100644 --- a/Misc/NEWS.d/next/Library/2018-10-15-23-56-39.bpo-34995.kNA9ge.rst +++ b/Misc/NEWS.d/next/Library/2018-10-15-23-56-39.bpo-34995.kNA9ge.rst @@ -1 +1 @@ -Maintain wrapped method's __isabstractmethod__ in functools.cached_property +:func:`functools.cached_property` now also copies the wrapped method's ``__isabstractmethod__`` attribute. From 511c14f2fe433d8ceb517e06f78f3ee6c66d0d06 Mon Sep 17 00:00:00 2001 From: Matt Wilber Date: Fri, 26 Oct 2018 05:41:23 +0000 Subject: [PATCH 6/6] Avoid long lines --- Lib/test/test_functools.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index 46bd7cab93e78e..fbfff79a66af81 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -2485,7 +2485,8 @@ class AbstractExpensiveCalculator(abc.ABC): def calculate(self): pass - self.assertTrue(AbstractExpensiveCalculator.calculate.__isabstractmethod__) + method = AbstractExpensiveCalculator.calculate + self.assertTrue(method.__isabstractmethod__) with self.assertRaises(TypeError): AbstractExpensiveCalculator() @@ -2495,7 +2496,8 @@ class ConcreteExpensiveCalculator(abc.ABC): def calculate(self): return 42 - self.assertFalse(getattr(ConcreteExpensiveCalculator.calculate, '__isabstractmethod__', False)) + method = ConcreteExpensiveCalculator.calculate + self.assertFalse(getattr(method, '__isabstractmethod__', False)) ConcreteExpensiveCalculator()