Subclass of property doesn't preserve instance __doc__ when using doc= argument

[embray]

BPO	24766
Nosy	@bitdancer, @Bluehorn, @ethanfurman, @embray, @berkerpeksag, @iritkatriel
PRs	bpo-24766: doc= argument to subclasses of property not handled correctly #2487
Files	property-doc.patch: Initial proposed patch property-doc-test.patch property-doc-test2.patch issue24766.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2015-07-31.17:02:03.577>
labels = ['interpreter-core', 'type-bug', '3.11']
title = "Subclass of property doesn't preserve instance __doc__ when using doc= argument"
updated_at = <Date 2021-10-21.15:30:13.977>
user = 'https://github.com/embray'

bugs.python.org fields:

activity = <Date 2021-10-21.15:30:13.977>
actor = 'iritkatriel'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2015-07-31.17:02:03.577>
creator = 'erik.bray'
dependencies = []
files = ['40086', '40181', '40504', '42683']
hgrepos = []
issue_num = 24766
keywords = ['patch']
message_count = 12.0
messages = ['247756', '247758', '248604', '248606', '250937', '250990', '250994', '250997', '251006', '264642', '264646', '404608']
nosy_count = 6.0
nosy_names = ['r.david.murray', 'torsten', 'ethan.furman', 'erik.bray', 'berker.peksag', 'iritkatriel']
pr_nums = ['2487']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue24766'
versions = ['Python 3.11']

[embray]

This issue is directly related to http://bugs.python.org/issue5890, the solution to which I think was incomplete.

The examples below use a trivial subclass of property (but apply as well to a less trivial one):

>>> class myproperty(property): pass
...

When using myproperty with the decorator syntax, or simply without specifying a doc= argument, the docstring is properly inherited from the getter, as was fixed by bpo-5890:

>>> class A:
...     @myproperty
...     def foo(self):
...         """The foo."""
...         return 1
... 
>>> A.foo.__doc__
'The foo.'

However, when using the doc= argument, this behavior is broken:

>>> class B:
...     def _get_foo(self): return 1
...     foo = myproperty(_get_foo, doc="The foo.")
... 
>>> B.foo.__doc__
>>> B.foo.__doc__ is None
True

The attached patch resolves the issue by applying the special case for subclasses more generally. If this looks good I'll add a test as well.

One thing I went back and forth on in the "if (Py_TYPE(self) != &PyProperty_Type)" block was whether or not to then deref prop->prop_doc and set it to NULL, since I don't think it's needed anymore at this point. But I decided it was ultimately harmless to leave it.

[ethanfurman]

Looks good so far. Let's get some tests in place.

[embray]

Sorry for the hold up. Attached is another diff providing a test. I think this is all that's really needed (since this is just a special case of the issue already tested for in this particular test class.

[ethanfurman]

Larry, can we get this into 3.5? I'll create a pull-request in a couple days.

[ethanfurman]

ethan@code:~/source/python/issue25147$ ./python -m test.regrtest -R3:3 test_property
[1/1] test_property

Eggs # from print() inside test

beginning 6 repetitions
123456

Spam # the new value from the previous run seems stuck

test test_property failed -- Traceback (most recent call last):
  File "/home/ethan/source/python/issue25147/Lib/test/test_property.py", line 175, in test_property_decorator_doc_writable
    self.assertEqual(sub.__class__.spam.__doc__, 'Eggs')
AssertionError: 'Spam' != 'Eggs'
- Spam
+ Eggs

1 test failed:
test_property

[embray]

Interesting--in fairness to myself, that test seems to fail without my patch too (without the -R it passes, but with -R3:3 it fails). So it looks like a pre-existing issue. But I'll see if I can do something about that while I'm at it.

[embray]

Actually, of course that test would fail when run repeatedly--it sets the property docstring from 'Eggs' to 'Spam' on the first run, but then doesn't set it back to its original value. Since the PropertyWritableDocs class used in that test is module-level it doesn't get reset.

I'd just update that test to return the docstring to its original value if you want it to pass under such a condition.

[embray]

Attached an additional patch to test_property_decorator_doc_writable so that it can pass on repeated runs.

[ethanfurman]

Thanks for checking that out, Erik. I was hoping it was a testing issue, but I ran out of time to verify.

[berkerpeksag]

test_property_decorator_doc_writable fix has already been committed in cc1aa0e88626.

Here's an updated patch:

• Synced with the default branch
• Adapted the test from duplicate bpo-25757 (written by Torsten Landschoff)

[embray]

Thanks for the updated patch. LGTM.

[iritkatriel]

Reproduced on 3.11.

[embray]

@serhiy-storchaka thanks for helping get this done. I've been totally inactive in core Python development for some years now and forgot about this. Always fun to see some very minor bug reported 9 years ago fixed 🙃

[serhiy-storchaka]

You are welcome. I'm working on pushing old forgotten PRs.