Repository navigation
inspect.getmembers and inspect.classify_class_attrs mishandle descriptors #63230
Description
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 = 'https://github.com/ethanfurman'
closed_at = <Date 2013-10-18.07:28:05.769>
created_at = <Date 2013-09-16.00:00:54.563>
labels = ['type-bug']
title = 'inspect.getmembers and inspect.classify_class_attrs mishandle descriptors'
updated_at = <Date 2014-02-25.21:05:20.551>
user = 'https://github.com/ethanfurman'bugs.python.org fields:
activity = <Date 2014-02-25.21:05:20.551>
actor = 'r.david.murray'
assignee = 'ethan.furman'
closed = True
closed_date = <Date 2013-10-18.07:28:05.769>
closer = 'python-dev'
components = []
creation = <Date 2013-09-16.00:00:54.563>
creator = 'ethan.furman'
dependencies = []
files = ['31787', '31797', '31812', '31846', '32123', '32165', '32166', '32275', '32297']
hgrepos = []
issue_num = 19030
keywords = ['patch']
message_count = 42.0
messages = ['197844', '197853', '197854', '197855', '197857', '197861', '197867', '197871', '197879', '197883', '197884', '197891', '197902', '197906', '198037', '198306', '198313', '198387', '198481', '198506', '199736', '199920', '199929', '200186', '200194', '200230', '200699', '200726', '200768', '200772', '200803', '200805', '200807', '200815', '200824', '200915', '200949', '200952', '200959', '200963', '212211', '212212']
nosy_count = 11.0
nosy_names = ['barry', 'mjpieters', 'ncoghlan', 'pitrou', 'r.david.murray', 'eli.bendersky', 'skrah', 'neologix', 'ethan.furman', 'python-dev', 'koobs']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue19030'
versions = ['Python 3.4']
Activity
[-]inspect.getmembers and inspect.classify_class_attrs mishandle descriptors[/-][+]inspect.getmembers and inspect.classify_class_attrs mishandle descriptors[/+]ethanfurman commentedon Sep 16, 2013
Switching the order to try getattr first is going to make getting the doc from the descriptor problematic -- we have no way of knowing if the descriptor doc goes with the object we got back from getattr.
Current patch adds VirtualAttribute to types, and reworks inspect.classify_class_attrs, inspect.getmembers, and Enum to use that instead of _RouteClassAttributeToGetattr (which has been removed).
Tests are still needed for the new VirtualAttribute.
Not sure VirtualAttribute is the best name; other ideas:
ncoghlan commentedon Sep 16, 2013
The current behaviour is broken for *any* descriptor which doesn't
return itself when retrieved from the class. It just so happens that
all the *other* descriptors in the standard library work that way, so
it doesn't matter if you retrieve them directly from __dict__ or
retrieve them with getattr - you'll get the descriptor object either
way.
So we should be calling getattr first, and always taking __doc__ (if
any) from the returned object.
ethanfurman commentedon Sep 19, 2013
Okay, some slight reorganizing. Current patch gives preferential treatment to getattr, falling back on dict lookup if getattr raises an exception or if the resulting object's class is not found in the mro (including the metaclass mro).
Amazingly, nothing appeared to break in the test suite. :)
If no kibitzing in the next couple days I'll write some tests to better excercise the metaclass portion (I used Enum for interactive testing).
ethanfurman commentedon Sep 22, 2013
Current patch has a little more code cleanup and a bunch more tests.
I copied and adapted test_property to test_VirtualAttribute, and VirtualAttribute passes every test except the __slots__ test where __doc__ is not supposed to copy. I think the problem there is that VA is a python object and already has a __doc__ so having __slots__ not show __doc__ doesn't help. I put a skipIf(hasattr, obj, '__doc__') around that test.
I'm still not crazy about the name VirtualAttribute... oh well.
If no negative feedback by mid-week I'll commit, as I would really like this code to be the next alpha.
ncoghlan commentedon Sep 23, 2013
I suggest DynamicClassAttribute for the descriptor name.
python-dev commentedon Sep 25, 2013
New changeset 436b606ecfe8 by Ethan Furman in branch 'default':
Close bpo-19030: improvements to inspect and Enum.
http://hg.python.org/cpython/rev/436b606ecfe8
mjpieters commentedon Sep 27, 2013
Note: there is a comment explaining the point of _RouteClassAttributeToGetattr right above the Enum.name and Enum.value methods you changed (now at line 474). You may want to update that comment now.
python-dev commentedon Sep 28, 2013
New changeset 96081e7526f0 by Ethan Furman in branch 'default':
bpo-19030: fixed comment that was still referring to a changed descriptor.
http://hg.python.org/cpython/rev/96081e7526f0
python-dev commentedon Oct 13, 2013
New changeset 3752c94368dd by Ethan Furman in branch 'default':
bpo-19030: commit tests for DynamicClassAttribute
http://hg.python.org/cpython/rev/3752c94368dd
ethanfurman commentedon Oct 14, 2013
Updated and renamed the DynamicClassAttribute tests, and discovered that classify_class_attrs is not handling instance portion correctly.
We should see both eggs and spam, but only eggs is showing up.
ethanfurman commentedon Oct 14, 2013
Updated tests now passing.
Will commit Thursday, or Friday at the latest.
python-dev commentedon Oct 18, 2013
New changeset 39b06c3fbe2e by Ethan Furman in branch 'default':
Close bpo-19030: inspect.getmembers and inspect.classify_class_attrs
http://hg.python.org/cpython/rev/39b06c3fbe2e
koobs commentedon Oct 18, 2013
Multiple test_pydoc failures found on koobs-freebsd* buildbots after 39b06c3fbe2e6ef78a540513d4b81f2d095d1e62
Attaching complete logs from both bots to this issue, will reference bpo-16938 as well
python-dev commentedon Oct 18, 2013
New changeset 02c9d26d231f by Ethan Furman in branch 'default':
Issue bpo-19030: special-cased __dict__ as the actual dict is not returned, a proxy is.
http://hg.python.org/cpython/rev/02c9d26d231f
python-dev commentedon Oct 21, 2013
New changeset 2f09a6980e1a by Ethan Furman in branch 'default':
Issue bpo-19030: final pieces for proper location of various class attributes located in the metaclass.
http://hg.python.org/cpython/rev/2f09a6980e1a
koobs commentedon Oct 21, 2013
@Ethan, not sure if you've already seen them, but there are 4 pydoc failures since 2f09a6980e1a
Attaching another complete log from build #245 on the koobs-freebsd9 buildslave here for posterity
python-dev commentedon Oct 21, 2013
New changeset dad1debba93c by Charles-François Natali in branch 'default':
Fix test_pydoc failure introduced by 2f09a6980e1a (issue bpo-19030).
http://hg.python.org/cpython/rev/dad1debba93c
neologix commentedon Oct 21, 2013
I took the freedom to push a fix for the test_pydoc failures (all the buildbots were red). It should fix the failures, but since I'm not familiar with the code, it'd be good to have someone double-check it.
Ethan, you just broke all the buildbots: you're now officialy a core developer! ;-)
skrah commentedon Oct 21, 2013
The build --without-doc-strings still fails on the FreeBSD-9.0 bot.
Antoine, could we make that option official on my build slave? Currently
I'm subverting the build system by exporting an environment variable.
It does not seem to be a heavy maintenance burden: This is the first
failure in half a year or so.
ethanfurman commentedon Oct 21, 2013
Charles-François Natali added the comment:
I was actually hoping to put off this particular honor for a while... drat! :)
ethanfurman commentedon Oct 21, 2013
Thanks, Charles-François!
The problem occurred when I tried to push the commit and was told there was trailing white-space. Naturally I then ran the de-whitespacing tool which of course removed the whitespace from those lines where you added the \x20s back on. I'll remember to do those conversions next time, and thanks for fixing it -- I'm not sure I would have found it any time soon.
pitrou commentedon Oct 21, 2013
Done. Can you please watch for failures and ensure they get fixed?
skrah commentedon Oct 21, 2013
Antoine Pitrou <report@bugs.python.org> wrote:
Thanks! Yes, I'll keep an eye on it.
skrah commentedon Oct 22, 2013
Here's the fix for --without-doc-strings (can't commit right now).
ethanfurman commentedon Oct 22, 2013
Stefan, do the other tests in PydocWithMetaClasses continue to work on your FreeBSD 9.0 box without docstrings? Because they all fail on my Linux Ubuntu 13.04 box.
skrah commentedon Oct 22, 2013
Ethan Furman <report@bugs.python.org> wrote:
I tested on Debian, and the remaining tests seem to work (I did not bother to
figure out why though). Let's just skip all tests if they fail for you.
ethanfurman commentedon Oct 22, 2013
Actually, you @skipif clued me in as to what was happening with the other pydoc tests and their skipif clauses.
Added appropriate skipifs to the new tests and mimicked the docstring in/exclusion. All tests now passing with and without docstrings, with and without -OO (simply skipped with -OO).
python-dev commentedon Oct 22, 2013
New changeset 64d94b21e731 by Ethan Furman in branch 'default':
Issue bpo-19030: fix new pydoc tests for --without-doc-strings
http://hg.python.org/cpython/rev/64d94b21e731
python-dev commentedon Feb 25, 2014
New changeset 4cd620d8c3f6 by R David Murray in branch 'default':
whatsnew: DynanicClassAttribute (bpo-19030), Py_SetStandardStreamEncoding (bpo-16129)
http://hg.python.org/cpython/rev/4cd620d8c3f6
bitdancer commentedon Feb 25, 2014
I added docs for DynamicClassAttribute by copying the docstring. I think the doc entry could use some expansion, though, as it isn't obvious how to use it (or what, in fact, it does exactly).