Fix OrderedDic.pop() to work for subclasses that define __missing__().

Lib/collections.py

@@ -22,7 +22,7 @@ from reprlib import recursive_repr as _recursive_repr
 class _Link(object):
     __slots__ = 'prev', 'next', 'key', '__weakref__'
 
-class OrderedDict(dict, MutableMapping):
+class OrderedDict(dict):
     'Dictionary that remembers insertion order'
     # An inherited dict maps keys to values.
     # The inherited dict provides __getitem__, __len__, __contains__, and get.
@@ -172,12 +172,22 @@ class OrderedDict(dict, MutableMapping):
         return size
 
     update = __update = MutableMapping.update
-    pop = MutableMapping.pop
     keys = MutableMapping.keys
     values = MutableMapping.values
     items = MutableMapping.items
     __ne__ = MutableMapping.__ne__
 
+    __marker = object()
+
+    def pop(self, key, default=__marker):
+        if key in self:
+            result = self[key]
+            del self[key]
+            return result
+        if default is self.__marker:
+            raise KeyError(key)
+        return default
+
     def setdefault(self, key, default=None):
         'OD.setdefault(k[,d]) -> OD.get(k,d), also set OD[k]=d if k not in OD'
         if key in self:

Lib/test/test_collections.py

@@ -834,6 +834,10 @@ class TestOrderedDict(unittest.TestCase):
         self.assertEqual(list(d.items()),
             [('a', 1), ('b', 2), ('c', 3), ('d', 4), ('e', 5), ('f', 6), ('g', 7)])
 
+    def test_abc(self):
+        self.assertIsInstance(OrderedDict(), MutableMapping)
+        self.assertTrue(issubclass(OrderedDict, MutableMapping))
+
     def test_clear(self):
         pairs = [('c', 1), ('b', 2), ('a', 3), ('d', 4), ('e', 5), ('f', 6)]
         shuffle(pairs)
@@ -892,6 +896,17 @@ class TestOrderedDict(unittest.TestCase):
         self.assertEqual(len(od), 0)
         self.assertEqual(od.pop(k, 12345), 12345)
 
+        # make sure pop still works when __missing__ is defined
+        class Missing(OrderedDict):
+            def __missing__(self, key):
+                return 0
+        m = Missing(a=1)
+        self.assertEqual(m.pop('b', 5), 5)
+        self.assertEqual(m.pop('a', 6), 1)
+        self.assertEqual(m.pop('a', 6), 6)
+        with self.assertRaises(KeyError):
+            m.pop('a')
+
     def test_equality(self):
         pairs = [('c', 1), ('b', 2), ('a', 3), ('d', 4), ('e', 5), ('f', 6)]
         shuffle(pairs)