[Patches] [ python-Patches-539949 ] dict.popitem(key=None)

[noreply@sourceforge.net]

Patches item #539949, was opened at 2002-04-05 14: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: Tim Peters (tim_one)
Date: 2002-04-05 16: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 16: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 16: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 16: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 16: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 15: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