Issue1826
Created on 2008-01-14 19:22 by barry, last changed 2008-02-23 23:02 by georg.brandl.
| File name |
Uploaded |
Description |
Edit |
Remove |
|
getattrchaser.diff
|
scottdial,
2008-01-14 23:59
|
patch to add recursive chasing of attributes in operator.attrgetter |
|
|
|
getattrchaser.diff
|
scottdial,
2008-01-15 13:54
|
(updated) patch to add recursive chasing of attributes in operator.attrgetter |
|
|
|
attrgetter-2.diff
|
georg.brandl,
2008-01-19 23:40
|
|
|
|
| msg59911 (view) |
Author: Barry A. Warsaw (barry) |
Date: 2008-01-14 19:22 |
|
operator.attrgetter() is neutered because it only accepts a single
attribute name, but it would really be much more useful if it accepted,
and followed a dotted attribute path, e.g.:
sorted(seq, key=operator.attrgetter('person.displayname'))
Of course attrgetter() would raise AttributeError if the entire path
could not be traversed.
|
| msg59914 (view) |
Author: Guido van Rossum (gvanrossum) |
Date: 2008-01-14 19:35 |
|
How about a slight change to the proposed API, and make it a varargs
function? E.g.
operator.attrgetter('person', 'displayname')
This would avoid the need to parse the argument.
|
| msg59915 (view) |
Author: Barry A. Warsaw (barry) |
Date: 2008-01-14 19:40 |
|
Unfortunately, that already has different, existing (and IMHO less
useful) semantics. :(
|
| msg59916 (view) |
Author: Guido van Rossum (gvanrossum) |
Date: 2008-01-14 19:41 |
|
Fair enough. Fine with me then.
|
| msg59917 (view) |
Author: Raymond Hettinger (rhettinger) |
Date: 2008-01-14 20:42 |
|
Mind if I take a look at the patch before it goes in?
|
| msg59918 (view) |
Author: Barry A. Warsaw (barry) |
Date: 2008-01-14 20:45 |
|
I don't even mind if you *write* the patch :)
I actually haven't had time yet to hack it up, but I just reached my
threshold of tolerance so I had to at least report the bug. I think
it's always a good idea to review patches before they go in anyway, so
if I work one up, I'll attach it here.
|
| msg59919 (view) |
Author: Barry A. Warsaw (barry) |
Date: 2008-01-14 20:47 |
|
Hey Guido, can I borrow the keys to the time machine to get this into
Python 2.5.0? :)
|
| msg59927 (view) |
Author: Martin v. Löwis (loewis) |
Date: 2008-01-14 22:03 |
|
What's wrong with
sorted(seq, key=lambda v:v.person.displayname)
|
| msg59930 (view) |
Author: Scott Dial (scottdial) |
Date: 2008-01-14 22:25 |
|
I want to clarify that the proposed change would break:
operator.attrgetter(foo)(bar) == getattr(bar, foo)
Which is the documented intent of the operator module: "This module
exports a set of functions implemented in C corresponding to the
intrinsic operators of Python."
Unless, you are proposing getattr grow this functionality as well. IOW,
should PyObject_GetAttr* perform this recursion? If so, should
PyObject_HasAttr*, PyObject_DelAttr*, PyObject_SetAttr* do this as well?
At the moment, it is possible to have foo's (as in above) that contain
dots despite the inability to spell that in python's dereferencing
syntax. However, I don't know that anybody does that.
|
| msg59934 (view) |
Author: Barry A. Warsaw (barry) |
Date: 2008-01-14 23:05 |
|
I'm not /personally/ concerned with the breakage because practicality
beats purity, and I don't want to use lambda because it's slower. I've
never used operator.attrgetter() outside the specific use case of
sorted() and list.sort() so I'd like to make it able to be used in all
sorting-by-attribute-chasing use cases.
Other options: have sorted() and list.sort() grow another keyword
argument for the attribute-path; add another method called attrchaser()
in some module that adds the requested functionality.
|
| msg59936 (view) |
Author: Raymond Hettinger (rhettinger) |
Date: 2008-01-14 23:12 |
|
I think Barry's proposal is fine. If we need to update the docs, then
so be it. His proposal adds useful functionality while keeping
readability and clarity.
FWIW, the getattr() explanation in the docs already needs to be expanded
to cover the multi-argument form of itemgetter.
|
| msg59940 (view) |
Author: Scott Dial (scottdial) |
Date: 2008-01-14 23:59 |
|
The attached patch provides for the functionality requested. I've
updated the docstring of attrgetter related to this new feature and have
updated test_operator accordingly.
|
| msg59965 (view) |
Author: Scott Dial (scottdial) |
Date: 2008-01-15 13:54 |
|
Mea culpa, the original patch I attached here has an obvious duplication
of code in test_operator.py. I've attached an updated patch to make life
easier on the commiter.
|
| msg60249 (view) |
Author: Georg Brandl (georg.brandl) |
Date: 2008-01-19 23:40 |
|
Attaching another patch that doesn't create a list of strings every time
the attrgetter is called. Also includes docs.
|
| msg61296 (view) |
Author: Antoine Pitrou (pitrou) |
Date: 2008-01-20 14:14 |
|
FWIW, Georg's patch works here.
Perhaps there should be a test for unicode attribute names since it is
special-cased in the patch.
|
| msg62834 (view) |
Author: Georg Brandl (georg.brandl) |
Date: 2008-02-23 23:02 |
|
Committed in r61027.
|
|
| Date |
User |
Action |
Args |
| 2008-02-23 23:02:30 | georg.brandl | set | status: open -> closed messages:
+ msg62834 |
| 2008-01-20 14:14:30 | pitrou | set | nosy:
+ pitrou messages:
+ msg61296 |
| 2008-01-19 23:40:38 | georg.brandl | set | files:
+ attrgetter-2.diff nosy:
+ georg.brandl messages:
+ msg60249 |
| 2008-01-15 13:54:17 | scottdial | set | files:
+ getattrchaser.diff messages:
+ msg59965 |
| 2008-01-14 23:59:41 | scottdial | set | files:
+ getattrchaser.diff messages:
+ msg59940 |
| 2008-01-14 23:12:22 | rhettinger | set | resolution: accepted messages:
+ msg59936 |
| 2008-01-14 23:05:36 | barry | set | messages:
+ msg59934 |
| 2008-01-14 22:25:44 | scottdial | set | nosy:
+ scottdial messages:
+ msg59930 |
| 2008-01-14 22:03:30 | loewis | set | nosy:
+ loewis messages:
+ msg59927 |
| 2008-01-14 20:47:27 | barry | set | messages:
+ msg59919 |
| 2008-01-14 20:45:43 | barry | set | messages:
+ msg59918 |
| 2008-01-14 20:42:50 | rhettinger | set | nosy:
+ rhettinger messages:
+ msg59917 |
| 2008-01-14 19:41:56 | gvanrossum | set | messages:
+ msg59916 |
| 2008-01-14 19:40:31 | barry | set | messages:
+ msg59915 |
| 2008-01-14 19:35:50 | gvanrossum | set | nosy:
+ gvanrossum messages:
+ msg59914 |
| 2008-01-14 19:22:19 | barry | create | |
|