[Patches] [ python-Patches-539949 ] dict.popitem(key=None)
noreply@sourceforge.net
noreply@sourceforge.net
Sat, 06 Apr 2002 17:08:38 -0800
Patches item #539949, was opened at 2002-04-05 19:38
You can respond by visiting:
http://sourceforge.net/tracker/?func=detail&atid=305470&aid=539949&group_id=5470
Category: Core (C code)
Group: Python 2.3
Status: Open
Resolution: Accepted
Priority: 5
Submitted By: Raymond Hettinger (rhettinger)
Assigned to: Guido van Rossum (gvanrossum)
Summary: dict.popitem(key=None)
Initial Comment:
This patch implements the feature request at
http://sourceforge.net/tracker/index.php?
func=detail&aid=495086&group_id=5470&atid=355470 which
asks for an optional argument to popitem so that it
returns a key/value pair for a specified key or, if
not specified, an arbitrary key.
The benefit is in providing a fast, explicit way to
retrieve and remove and particular key/value pair from
a dictionary. By using only a single lookup, it is
faster than the usual Python code:
value = d[key]
del d[key]
return (key, value)
which now becomes:
return d.popitem(key)
There is no magic or new code in the implementation --
it uses a few lines each from getitem, delitem, and
popitem. If an argument is specified, the new code is
run; otherwise, the existing code is run. This
assures that the patch does not cause a performance
penalty.
The diff is about -3 lines and +25 lines.
There are four sections:
1. Replacement code for dict_popitem in dictobject.c
2. Replacement docstring for popitem in dictobject.c
3. Replacement registration line for popitem in
dictobject.c
4. Sample Python test code.
----------------------------------------------------------------------
>Comment By: Raymond Hettinger (rhettinger)
Date: 2002-04-07 01:08
Message:
Logged In: YES
user_id=80475
The tests and documentation patches have been added.
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2002-04-06 17:23
Message:
Logged In: YES
user_id=80475
Q: Does the new function signature slow the existing no
argument case? A: Yes. The function is already so fast,
that the small overhead of PyArg_ParseTuple is measurable.
My timing shows a 8% drop in speed.
Q: Is _,v=d.popitem(k) slower than v=d.popvalue(k)? A:
Yes. Though popvalue is a non-existing strawman, it would
be quicker: it would cost two calls to Py_DECREF while
saving a call to PyTuple_New and two calls to
PyTuple_SET_ITEM. Still, the running time for popvalue
would be dominated by the rest of the function and not the
single malloc. Also, I think it unlikely that the
dictionary interface would ever be expanded for popvalue,
so the comparison is moot.
Q: Are there cases where (k,v) is needed? A: Yes. One
common case is where the tuple still needs to be formed to
help build another dictionary: dict([d.popitem(k) for k in
xferlist]) or [n.__setitem__(d.popitem(k)) for k in
xferlist].
Also, it is useful when the key is computed by a function
and then needs to be used in an expression. I often do
something like that with setdefault: uniqInOrder=
[u.setdefault(k,k) for k in alist if k not in u].
Also, when the key is computed by a function, it may need
to be saved only when .popitem succeeds but not when the
key is missing: "get and remove key if present; trigger
exception if absent" This pattern is used in validating
user input keys for deletion.
Q: Where is the unittest and doc patch? A: Coming this
weekend.
----------------------------------------------------------------------
Comment By: Tim Peters (tim_one)
Date: 2002-04-05 21:50
Message:
Logged In: YES
user_id=31435
Are there examples of concrete use cases? The idea that
dict.popitem(k) returns (k, dict[k]) seems kinda goofy,
since you necessarily already have k.
So the question is whether this is the function signature
that's really desired, or whether it's too much a hack. As
is, it slows down popitem() without an argument because it
requires using a fancier calling sequence, and because it
now defers that case to a taken branch; it's also much
slower than a function that just returned v could be, due
to the need to allocate a 2-tuple to hold a redundant copy
of the key.
Perhaps there are use cases of the form
k, v = dict.popitem(f(x, y, z))
where the key is known only implicitly?
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2002-04-05 21:47
Message:
Logged In: YES
user_id=6380
FYI, I'm uploading my version of the patch, with code
cleanup, as popdict2.txt. I've moved the popitem-with-arg
code before the allocation of res, because there were
several places where this code returned NULL without
DECREF'ing res. Repeating the PyTuple_New(2) call seemed the
lesser evil.
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2002-04-05 21:38
Message:
Logged In: YES
user_id=6380
I've reviewed the patch and see only cosmetic things that
need to be changed. I'll check it in as soon as you submit a
unittest and doc patch.
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2002-04-05 21:26
Message:
Logged In: YES
user_id=6380
Now, if you could also upload a unittest and a doc patch,
that would be great!
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2002-04-05 21:10
Message:
Logged In: YES
user_id=80475
Context diff uploaded at poppatch.c below.
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2002-04-05 20:11
Message:
Logged In: YES
user_id=6380
Please upload a context or unified diff.
----------------------------------------------------------------------
You can respond by visiting:
http://sourceforge.net/tracker/?func=detail&atid=305470&aid=539949&group_id=5470