Skip to content

gh-83888: Document that a concrete override need not keep abstract components - #158131

Open
v0ropaev wants to merge 3 commits into
python:mainfrom
v0ropaev:gh-83888-abstract-property-docs
Open

v0ropaev wants to merge 3 commits into
python:mainfrom
v0ropaev:gh-83888-abstract-property-docs

Conversation

@v0ropaev

@v0ropaev v0ropaev commented Sep 24, 2026 •

Copy link
Copy Markdown

Closes #83888, on @gvanrossum's invitation in that thread ("Josh, feel free to submit a PR (make sure it mentions this issue)").

The abc docs let a reader conclude that overriding an abstract read-write property obliges the subclass to supply a setter. It does not. Measured on main:

>>> class C(metaclass=abc.ABCMeta):
...     @property
...     @abc.abstractmethod
...     def foo(self): ...
...     @foo.setter
...     @abc.abstractmethod
...     def foo(self, v): ...
...
>>> C.__abstractmethods__
frozenset({'foo'})
>>> class D(C):
...     @property
...     def foo(self): return 3        # read-only, no setter
...
>>> D.__abstractmethods__
frozenset()
>>> D().foo
3
>>> D().foo = 4
AttributeError: property 'foo' of 'D' object has no setter

So D instantiates happily and the missing setter surfaces much later, as an AttributeError at the assignment.

Why, and why this is a docs change

@rhettinger explained the mechanism and ruled out fixing it:

Right now, ABCMeta only looks at the concrete property. It would have to be modified to scan the next in MRO for an abstract property and then pick apart its component fget, fset, and fdel. That would be a significant jump in complexity with only a minimal payoff.

and @gvanrossum agreed and closed it on that basis:

I agree with Raymond here. This is a job for a static type checker like mypy.

The issue survived as a documentation one, so the text now describes what happens and names that remedy.

Wording

The mechanism is stated as what it actually is, which is not specific to properties: ABCMeta inspects only the object a subclass finally binds to an abstract name, and if that object does not report itself as abstract, the name counts as implemented. Binding foo = 3 clears the abstraction just as effectively as a read-only property does — a property-shaped explanation would have been too narrow.

The abstractproperty block gets a matching sentence, since it is the one place that demonstrated borrowing the remaining components from the base class and so reads as if that were required.

Test

test_concrete_override_drops_abstract_components pins the documented behaviour: the read-only override, its later AttributeError, and the plain-value override. Without it the prose would be the only record of a behaviour nobody has asserted.

It is not tautological — changing the expected D.__abstractmethods__ back to {"foo"} gives 2 failures.

./python.exe -m test test_abc   →  run=76, SUCCESS

No runtime change; Doc/library/abc.rst and Lib/test/test_abc.py only, +45 lines, nothing removed. Since the diff includes a test file the docs label may not pull in skip news on its own — I do not think a Misc/NEWS.d entry is warranted for a docs-and-test change, but say the word and I will add one.

ABCMeta only inspects the object a subclass finally binds to an abstract
name; if that object does not report itself as abstract, the name counts
as implemented.  Nothing checks that a descriptor replacing an abstract
one still provides the same components, so a read-write abstract
property can be overridden by a read-only one without the subclass
losing its instantiability, and the missing setter surfaces only as an
AttributeError when the attribute is assigned to.

Document that in the abstractmethod section, and cancel the implicature
that was reported in the deprecated abstractproperty section by saying
outright that borrowing the remaining components from the base class is
not the only way to override an abstract property.
…nents (abc)

test_descriptors_with_abstractmethod only covers overrides built with
property.getter and property.setter, which carry the abstract components
of the base property forward.  Nothing pinned the behaviour documented
in the previous commit: that binding an abstract name to any
non-abstract object -- a read-only property, or a plain value -- makes
it concrete, whatever the original descriptor provided.
@bedevere-app

bedevere-app Bot commented Sep 24, 2026

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@bedevere-app bedevere-app Bot added tests Tests in the Lib/test dir awaiting review labels Sep 24, 2026
@read-the-docs-community

read-the-docs-community Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #34756980 | 📁 Comparing 68fbeb7 against main (a5b03fa)

  🔍 Preview build  

2 files changed
± library/abc.html
± whatsnew/changelog.html

@v0ropaev

Copy link
Copy Markdown
Author

Closing this myself. @picnixz pointed out on #158152 that I have been opening too many at once and that old issues nobody has revived are the wrong ones to pick up unasked — this is one of those, so it should not be sitting in the queue. The branch stays on my fork if the issue ever comes back to life.

@v0ropaev v0ropaev closed this Sep 25, 2026
@v0ropaev v0ropaev reopened this Sep 25, 2026
@bedevere-app

bedevere-app Bot commented Sep 25, 2026

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@v0ropaev

v0ropaev commented Sep 25, 2026 •

Copy link
Copy Markdown
Author

Reopened. Closing this was my own overreaction to being told I had too many open, not something anyone asked for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Abstract property setter/deleter implementation not enforced, but documented as such

1 participant