Issue 5890: Subclassing property doesn't preserve the auto doc behavior (original) (raw)

Created on 2009-04-30 22:13 by gsakkis, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
issue5890.patch r.david.murray,2009-05-04 21:09
issue5890-refix-py2.6.patch moriyoshi,2009-11-12 02:18 patch
issue5890-refix-trunk.patch moriyoshi,2009-11-12 02:18 patch (against trunk)
Messages (13)
msg86859 - (view) Author: George Sakkis (gsakkis) Date: 2009-04-30 22:13
Is the following behavior expected ? class MyProp(property): pass class Foo(object): @property def bar(self): '''Get a bar.''' @MyProp def baz(self): '''Get a baz.''' >>> print Foo.bar.__doc__ Get a bar. >>> print Foo.baz.__doc__ None
msg86918 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-05-01 21:53
What is happening here is that when __doc__ is looked up, it is found first among the attributes of the class type. __doc__ is special, and types define it to be None if it not set to anything specific. So the docstring for an instance of a subclass of property is None, or the docstring of the subclass if one is provided. The other property attributes aren't affected since they aren't "special" attributes on types, and so get looked up on the base class as expected. I believe the fix is to have property's __init__ check to see if it is dealing with a subclass, and if so to insert the __doc__ string into the instance's __dict__. Patch attached. Needs review, especially since I'm new to internals stuff.
msg86923 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-05-01 22:22
Fixes from review on #python-dev by Benjamin. It looks like I also need to look at property_copy.
msg87174 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-05-04 20:54
Updated patch that updates property_copy so that the __doc__ string is also copied appropriately when getter, setter, or deller are used. A bunch more tests, as well. I refactored property_copy to make it reuse the logic in property_init directly. Unfortunately I've got a refleak somewhere in there (regrtest -R :: says [8, 8, 8, 8]. Hopefully fresh and more experienced eyes can help me out.
msg87176 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-05-04 21:09
Updated patch with refleak fixed. Thanks Georg.
msg87199 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-05-05 01:03
Reviewed by Georg and approved on #python-dev. Committed in r72299, r72301, r72302, and r72304.
msg95142 - (view) Author: Moriyoshi Koizumi (moriyoshi) Date: 2009-11-11 17:09
A subclass of property doesn't always have writable __doc__, especially what is implemented in C. This actually causes a problem with Boost.Python's StaticProperty. References: - http://mail.python.org/pipermail/cplusplus-sig/2009-August/014747.html - http://lists.boost.org/Archives/boost/2009/10/157512.php - https://bugs.launchpad.net/ubuntu/+source/boost1.38/+bug/457688
msg95154 - (view) Author: Moriyoshi Koizumi (moriyoshi) Date: 2009-11-12 02:18
I created a patch against trunk and 2.6-maint that lets it simply ignore the error that might happen during PyObject_SetAttrString();
msg95155 - (view) Author: Moriyoshi Koizumi (moriyoshi) Date: 2009-11-12 02:18
and the other one
msg95319 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-11-16 01:08
It seems to me that all that patch does is re-enable the bug (in the extension class context) that the patch for this issue was a workaround for. I think perhaps the correct fix is to look at how the __doc__ attribute is found in the general case and see if it can be fixed there, allowing the removal of the workaround in property. See for example this sub-thread on python-dev: http://www.mail-archive.com/python-dev@python.org/msg42786.html
msg95345 - (view) Author: Moriyoshi Koizumi (moriyoshi) Date: 2009-11-16 13:28
Sorry, I don't quite have an idea on the part "these patches reenable the bug". The script of the first message yields exactly the same result both with the original 2.6.4 and the patched. Assuming the weirdness you referred to is different from 5890, I'd say the workaround introduced by the original patch to property class didn't correctly avoid the issue in the first place. I mean the fix should be fixed anyhow.
msg95346 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-11-16 13:51
Well, I haven't worked with boost or any other extension class that defines a subclass of property. If MyProp is such a subclass, would print Fro.baz.__doc__ print "Get a baz" in 2.6.2 but raise an error in 2.6.3/4, or would it print None? Regardless of which one it does, I think there is a better fix than patching the original patch in this issue, assuming I can code it and get approval to apply it. On the other hand, the suggested 'refix' may be appropriate for 2.6.5.
msg95464 - (view) Author: Moriyoshi Koizumi (moriyoshi) Date: 2009-11-19 05:28
@r.david.murray > If MyProp is such a subclass, would > print Fro.baz.__doc__ print "Get a baz" in 2.6.2 but raise an error in > 2.6.3/4, or would it print None? Just let it return None as they were for now. I completely agree there's a better fix like delegating the access to __doc__ to the base class (property class in this specific case).
History
Date User Action Args
2022-04-11 14:56:48 admin set github: 50140
2009-11-19 05:28:30 moriyoshi set messages: +
2009-11-16 13:51:44 r.david.murray set messages: +
2009-11-16 13:28:17 moriyoshi set messages: +
2009-11-16 01:08:51 r.david.murray set messages: +
2009-11-12 02🔞41 moriyoshi set files: + issue5890-refix-trunk.patchmessages: +
2009-11-12 02🔞05 moriyoshi set files: + issue5890-refix-py2.6.patchmessages: +
2009-11-11 17:09:18 moriyoshi set nosy: + moriyoshimessages: +
2009-05-05 01:03:19 r.david.murray set status: open -> closedresolution: fixedmessages: + stage: patch review -> resolved
2009-05-04 21:09:31 r.david.murray set files: - issue5890.patch
2009-05-04 21:09:26 r.david.murray set files: - issue5890.patch
2009-05-04 21:09:18 r.david.murray set files: + issue5890.patchmessages: +
2009-05-04 20:54:48 r.david.murray set files: + issue5890.patchmessages: +
2009-05-01 22:22:25 r.david.murray set files: + issue5890.patchmessages: +
2009-05-01 22:20:35 r.david.murray set files: - issue5890.patch
2009-05-01 21:53:17 r.david.murray set files: + issue5890.patchassignee: r.david.murraykeywords: + patch, needs reviewstage: patch reviewversions: + Python 2.6, Python 3.0, Python 3.1, Python 2.7nosy: + r.david.murraymessages: + priority: normalcomponents: + Interpreter Core
2009-04-30 22:13:11 gsakkis create