-
-
Notifications
You must be signed in to change notification settings - Fork 33.5k
bpo-41905: added abc.update_abstractmethods #22485
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.
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
Changes from 1 commit
f65b0f8
c04ba74
ea69ae6
8b76715
61c4f40
a5a9fd7
5e1fffc
2f95b84
3986c98
4f3d846
74473c8
92326c9
1e47ee4
740183f
114028e
ba8df08
29dba37
779e6cf
eea97f4
8eec6c2
5698153
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -121,6 +121,38 @@ def _abc_caches_clear(cls): | |||||||
| """Clear the caches (for debugging or testing).""" | ||||||||
| _reset_caches(cls) | ||||||||
|
|
||||||||
| def update_abstractmethods(cls): | ||||||||
bentheiii marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
| """Repopulate the abstract methods of an abstract class, or a subclass of | ||||||||
| an abstract class. | ||||||||
|
||||||||
| """Repopulate the abstract methods of an abstract class, or a subclass of | |
| an abstract class. | |
| """Recalculate the set of abstract methods of an abstract class. |
ericvsmith marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
ericvsmith marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
ericvsmith marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you didn't check in this function that it's an ABC, it's not guaranteed that cls.__abstractmethods__ works:
>>> class FooABC: pass
...
>>> FooABC.__abstractmethods__
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AttributeError: __abstractmethods__
Maybe add a test for when this function is called for a non-ABC.
bentheiii marked this conversation as resolved.
Show resolved
Hide resolved
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,7 +14,7 @@ | |
| 'partial', 'partialmethod', 'singledispatch', 'singledispatchmethod', | ||
| 'cached_property'] | ||
|
|
||
| from abc import get_cache_token | ||
| from abc import ABCMeta, get_cache_token, update_abstractmethods | ||
| from collections import namedtuple | ||
| # import types, weakref # Deferred to single_dispatch() | ||
| from reprlib import recursive_repr | ||
|
|
@@ -187,15 +187,20 @@ def _lt_from_ge(self, other, NotImplemented=NotImplemented): | |
|
|
||
| def total_ordering(cls): | ||
| """Class decorator that fills in missing ordering methods""" | ||
| # Find user-defined comparisons (not those inherited from object). | ||
| roots = {op for op in _convert if getattr(cls, op, None) is not getattr(object, op, None)} | ||
| # Find user-defined comparisons (not those inherited from object or abstract). | ||
| roots = {op for op in _convert | ||
| if getattr(cls, op, None) is not getattr(object, op, None) | ||
| and not getattr(getattr(cls, op, None), '__isabstractmethod__', False)} | ||
|
||
| if not roots: | ||
| raise ValueError('must define at least one ordering operation: < > <= >=') | ||
| root = max(roots) # prefer __lt__ to __le__ to __gt__ to __ge__ | ||
| for opname, opfunc in _convert[root]: | ||
| if opname not in roots: | ||
| opfunc.__name__ = opname | ||
| setattr(cls, opname, opfunc) | ||
| # update the abstract methods of the class, if it is abstract | ||
| if isinstance(cls, ABCMeta): | ||
| update_abstractmethods(cls) | ||
| return cls | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -487,6 +487,42 @@ class B: ... | |
| class C(with_metaclass(abc_ABCMeta, A, B)): | ||
| pass | ||
| self.assertEqual(C.__class__, abc_ABCMeta) | ||
|
|
||
| def test_update_new_abstractmethods(self): | ||
| class A(metaclass=abc_ABCMeta): | ||
| @abc.abstractmethod | ||
| def bar(self): | ||
| pass | ||
|
|
||
| @abc.abstractmethod | ||
| def updated_foo(self): | ||
| pass | ||
|
|
||
| A.foo = updated_foo | ||
| abc.update_abstractmethods(A) | ||
| msg = "class A with abstract methods bar, foo" | ||
| self.assertEqual(A.__abstractmethods__, {'foo', 'bar'}) | ||
| self.assertRaisesRegex(TypeError, msg, A) | ||
|
|
||
| def test_update_implementation(self): | ||
| class A(metaclass=abc_ABCMeta): | ||
| @abc.abstractmethod | ||
| def foo(self): | ||
| pass | ||
|
|
||
| class B(A): | ||
| pass | ||
|
|
||
| msg = "class B with abstract method foo" | ||
| self.assertRaisesRegex(TypeError, msg, B) | ||
| self.assertEqual(B.__abstractmethods__, {'foo'}) | ||
|
|
||
| B.foo = lambda self: None | ||
|
|
||
| abc.update_abstractmethods(B) | ||
|
|
||
| B() | ||
| self.assertEqual(B.__abstractmethods__, set()) | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you need this test case to cover the first loop:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @iritkatriel What does that test case accomplish other than showing that FooABC is now in an inconsistent state?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @gvanrossum In this comment I left out the update_abstractmethods, see Ben's version which he committed as test_update_del. Without this test the first loop in update_abstractmethods was not exercised. |
||
|
|
||
| class TestABCWithInitSubclass(unittest.TestCase): | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.