SF patch #693753: fix for bug 639806: default for dict.pop

(contributed by Michael Stone.)

6 files changed, 51 insertions, 11 deletions

diff --git a/Doc/lib/libstdtypes.tex b/Doc/lib/libstdtypes.tex
index 0006c76..1a7b505 100644
--- a/Doc/lib/libstdtypes.tex
+++ b/Doc/lib/libstdtypes.tex
@@ -1104,9 +1104,10 @@ arbitrary objects):
           {\code{\var{a}[\var{k}]} if \code{\var{k} in \var{a}},
            else \var{x} (also setting it)}
           {(5)}
-  \lineiii{\var{a}.pop(\var{k})}
-          {remove specified \var{key} and return corresponding \var{value}}
-          {}			     
+  \lineiii{\var{a}.pop(\var{k}\optional{, \var{x}})}
+          {\code{\var{a}[\var{k}]} if \code{\var{k} in \var{a}},
+           else \var{x} (and remove k)}
+          {(8)}
   \lineiii{\var{a}.popitem()}
           {remove and return an arbitrary (\var{key}, \var{value}) pair}
           {(6)}
@@ -1155,6 +1156,9 @@ over a dictionary, as often used in set algorithms.
 
 \item[(7)] \function{fromkeys()} is a class method that returns a
 new dictionary. \var{value} defaults to \code{None}.  \versionadded{2.3}
+
+\item[(8)] \function{pop()} raises a \exception{KeyError} when no default
+value is given and the key is not found.  \versionadded{2.3}
 \end{description}
 
 
diff --git a/Lib/UserDict.py b/Lib/UserDict.py
index fb9cdd5..6b5c9da 100644
--- a/Lib/UserDict.py
+++ b/Lib/UserDict.py
@@ -55,8 +55,8 @@ class UserDict:
         if not self.has_key(key):
             self[key] = failobj
         return self[key]
-    def pop(self, key):
-        return self.data.pop(key)
+    def pop(self, key, *args):
+        return self.data.pop(key, *args)
     def popitem(self):
         return self.data.popitem()
     def __contains__(self, key):
@@ -117,8 +117,16 @@ class DictMixin:
         except KeyError:
             self[key] = default
         return default
-    def pop(self, key):
-        value = self[key]
+    def pop(self, key, *args):
+        if len(args) > 1:
+            raise TypeError, "pop expected at most 2 arguments, got "\
+                              + repr(1 + len(args))
+        try:
+            value = self[key]
+        except KeyError:
+            if args:
+                return args[0]
+            raise
         del self[key]
         return value
     def popitem(self):
diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py
index d571a02..f2a7ccd 100644
--- a/Lib/test/test_types.py
+++ b/Lib/test/test_types.py
@@ -643,10 +643,14 @@ else: raise TestFailed, "{}.pop(k) doesn't raise KeyError when dictionary is emp
 # see SF bug #689659
 x = 4503599627370496L
 y = 4503599627370496
-h = {x: 'anything', y: 'something else'} 
+h = {x: 'anything', y: 'something else'}
 if h[x] != h[y]:
     raise TestFailed, "long/int key should match"
 
+if d.pop(k, v) != v: raise TestFailed, "{}.pop(k, v) doesn't return default value"
+d[k] = v
+if d.pop(k, 1) != v: raise TestFailed, "{}.pop(k, v) doesn't find known key/value pair"
+
 d[1] = 1
 try:
     for i in d:
diff --git a/Lib/test/test_userdict.py b/Lib/test/test_userdict.py
index d3e8002..cb98308 100644
--- a/Lib/test/test_userdict.py
+++ b/Lib/test/test_userdict.py
@@ -139,6 +139,9 @@ class UserDictTest(unittest.TestCase):
         t = UserDict.UserDict(x=42)
         self.assertEqual(t.pop("x"), 42)
         self.assertRaises(KeyError, t.pop, "x")
+        self.assertEqual(t.pop("x", 1), 1)
+        t["x"] = 42
+        self.assertEqual(t.pop("x", 1), 42)
 
         # Test popitem
         t = UserDict.UserDict(x=42)
@@ -242,6 +245,9 @@ class UserDictMixinTest(unittest.TestCase):
         self.assertEqual(s.pop(10), 'ten')
         self.assert_(10 not in s)
         s[10] = 'ten'
+        self.assertEqual(s.pop("x", 1), 1)
+        s["x"] = 42
+        self.assertEqual(s.pop("x", 1), 42)
 
         # popitem
         k, v = s.popitem()
diff --git a/Misc/NEWS b/Misc/NEWS
index 81df415..0a446df 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -12,6 +12,12 @@ What's New in Python 2.3 beta 1?
 Core and builtins
 -----------------
 
+- dict.pop now takes an optional argument specifying a default
+  value to return if the key is not in the dict.  If a default is not
+  given and the key is not found, a KeyError will still be raised.
+  Parallel changes were made to UserDict.UserDict and UserDict.DictMixin.
+  [SF patch #693753] (contributed by Michael Stone.)
+
 - sys.getfilesystemencoding() was added to expose
   Py_FileSystemDefaultEncoding.
 
diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index f7f2d6b..633f2ce 100644
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@ -1544,13 +1544,20 @@ dict_clear(register dictobject *mp)
 }
 
 static PyObject *
-dict_pop(dictobject *mp, PyObject *key)
+dict_pop(dictobject *mp, PyObject *args)
 {
 	long hash;
 	dictentry *ep;
 	PyObject *old_value, *old_key;
+	PyObject *key, *deflt = NULL;
 
+	if(!PyArg_UnpackTuple(args, "pop", 1, 2, &key, &deflt))
+		return NULL;
 	if (mp->ma_used == 0) {
+		if (deflt) {
+			Py_INCREF(deflt);
+			return deflt;
+		}
 		PyErr_SetString(PyExc_KeyError,
 				"pop(): dictionary is empty");
 		return NULL;
@@ -1563,6 +1570,10 @@ dict_pop(dictobject *mp, PyObject *key)
 	}
 	ep = (mp->ma_lookup)(mp, key, hash);
 	if (ep->me_value == NULL) {
+		if (deflt) {
+			Py_INCREF(deflt);
+			return deflt;
+		}
 		PyErr_SetObject(PyExc_KeyError, key);
 		return NULL;
 	}
@@ -1719,7 +1730,8 @@ PyDoc_STRVAR(setdefault_doc__,
 "D.setdefault(k[,d]) -> D.get(k,d), also set D[k]=d if k not in D");
 
 PyDoc_STRVAR(pop__doc__,
-"D.pop(k) -> v, remove specified key and return the corresponding value");
+"D.pop(k[,d]) -> v, remove specified key and return the corresponding value\n\
+If key is not found, d is returned if given, otherwise KeyError is raised");
 
 PyDoc_STRVAR(popitem__doc__,
 "D.popitem() -> (k, v), remove and return some (key, value) pair as a\n\
@@ -1763,7 +1775,7 @@ static PyMethodDef mapp_methods[] = {
 	 get__doc__},
 	{"setdefault",  (PyCFunction)dict_setdefault,   METH_VARARGS,
 	 setdefault_doc__},
-	{"pop",         (PyCFunction)dict_pop,          METH_O,
+	{"pop",         (PyCFunction)dict_pop,          METH_VARARGS,
 	 pop__doc__},
 	{"popitem",	(PyCFunction)dict_popitem,	METH_NOARGS,
 	 popitem__doc__},