ordereddict: simplify Python implementation

This is the first step in removing the C implementation of OrderedDict.

Rely on the ordering of the dict superclass. This fixes some of the
direct uses of dict methods on OrderedDict. Previously, these would
raise KeyError later on. Now they work.

Author: colesbury

Lib/collections/__init__.py

@@ -77,154 +77,32 @@ class _Link(object):
 
 class OrderedDict(dict):
     'Dictionary that remembers insertion order'
-    # An inherited dict maps keys to values.
-    # The inherited dict provides __getitem__, __len__, __contains__, and get.
-    # The remaining methods are order-aware.
-    # Big-O running times for all methods are the same as regular dictionaries.
-
-    # The internal self.__map dict maps keys to links in a doubly linked list.
-    # The circular doubly linked list starts and ends with a sentinel element.
-    # The sentinel element never gets deleted (this simplifies the algorithm).
-    # The sentinel is in self.__hardroot with a weakref proxy in self.__root.
-    # The prev links are weakref proxies (to prevent circular references).
-    # Individual links are kept alive by the hard reference in self.__map.
-    # Those hard references disappear when a key is deleted from an OrderedDict.
-
-    def __init__(self, other=(), /, **kwds):
-        '''Initialize an ordered dictionary.  The signature is the same as
-        regular dictionaries.  Keyword argument order is preserved.
-        '''
-        try:
-            self.__root
-        except AttributeError:
-            self.__hardroot = _Link()
-            self.__root = root = _proxy(self.__hardroot)
-            root.prev = root.next = root
-            self.__map = {}
-        self.__update(other, **kwds)
-
-    def __setitem__(self, key, value,
-                    dict_setitem=dict.__setitem__, proxy=_proxy, Link=_Link):
-        'od.__setitem__(i, y) <==> od[i]=y'
-        # Setting a new item creates a new link at the end of the linked list,
-        # and the inherited dictionary is updated with the new key/value pair.
-        if key not in self:
-            self.__map[key] = link = Link()
-            root = self.__root
-            last = root.prev
-            link.prev, link.next, link.key = last, root, key
-            last.next = link
-            root.prev = proxy(link)
-        dict_setitem(self, key, value)
-
-    def __delitem__(self, key, dict_delitem=dict.__delitem__):
-        'od.__delitem__(y) <==> del od[y]'
-        # Deleting an existing item uses self.__map to find the link which gets
-        # removed by updating the links in the predecessor and successor nodes.
-        dict_delitem(self, key)
-        link = self.__map.pop(key)
-        link_prev = link.prev
-        link_next = link.next
-        link_prev.next = link_next
-        link_next.prev = link_prev
-        link.prev = None
-        link.next = None
-
-    def __iter__(self):
-        'od.__iter__() <==> iter(od)'
-        # Traverse the linked list in order.
-        root = self.__root
-        curr = root.next
-        while curr is not root:
-            yield curr.key
-            curr = curr.next
-
-    def __reversed__(self):
-        'od.__reversed__() <==> reversed(od)'
-        # Traverse the linked list in reverse order.
-        root = self.__root
-        curr = root.prev
-        while curr is not root:
-            yield curr.key
-            curr = curr.prev
-
-    def clear(self):
-        'od.clear() -> None.  Remove all items from od.'
-        root = self.__root
-        root.prev = root.next = root
-        self.__map.clear()
-        dict.clear(self)
 
     def popitem(self, last=True):
         '''Remove and return a (key, value) pair from the dictionary.
 
         Pairs are returned in LIFO order if last is true or FIFO order if false.
         '''
+        if last:
+            return super().popitem()
         if not self:
             raise KeyError('dictionary is empty')
-        root = self.__root
-        if last:
-            link = root.prev
-            link_prev = link.prev
-            link_prev.next = root
-            root.prev = link_prev
-        else:
-            link = root.next
-            link_next = link.next
-            root.next = link_next
-            link_next.prev = root
-        key = link.key
-        del self.__map[key]
-        value = dict.pop(self, key)
-        return key, value
+        key = next(iter(self.keys()))
+        return (key, self.pop(key))
 
     def move_to_end(self, key, last=True):
         '''Move an existing element to the end (or beginning if last is false).
 
         Raise KeyError if the element does not exist.
         '''
-        link = self.__map[key]
-        link_prev = link.prev
-        link_next = link.next
-        soft_link = link_next.prev
-        link_prev.next = link_next
-        link_next.prev = link_prev
-        root = self.__root
+        value = self.pop(key)
         if last:
-            last = root.prev
-            link.prev = last
-            link.next = root
-            root.prev = soft_link
-            last.next = link
+            super().__setitem__(key, value)
         else:
-            first = root.next
-            link.prev = root
-            link.next = first
-            first.prev = soft_link
-            root.next = link
-
-    def __sizeof__(self):
-        sizeof = _sys.getsizeof
-        n = len(self) + 1                       # number of links including root
-        size = sizeof(self.__dict__)            # instance dictionary
-        size += sizeof(self.__map) * 2          # internal dict and inherited dict
-        size += sizeof(self.__hardroot) * n     # link objects
-        size += sizeof(self.__root) * n         # proxy objects
-        return size
-
-    update = __update = _collections_abc.MutableMapping.update
-
-    def keys(self):
-        "D.keys() -> a set-like object providing a view on D's keys"
-        return _OrderedDictKeysView(self)
-
-    def items(self):
-        "D.items() -> a set-like object providing a view on D's items"
-        return _OrderedDictItemsView(self)
-
-    def values(self):
-        "D.values() -> an object providing a view on D's values"
-        return _OrderedDictValuesView(self)
+            tmp = self.copy()
+            self.clear()
+            super().__setitem__(key, value)
+            self.update(tmp)
 
     __ne__ = _collections_abc.MutableMapping.__ne__
 
@@ -236,31 +114,17 @@ def pop(self, key, default=__marker):
         is raised.
 
         '''
-        marker = self.__marker
-        result = dict.pop(self, key, marker)
-        if result is not marker:
-            # The same as in __delitem__().
-            link = self.__map.pop(key)
-            link_prev = link.prev
-            link_next = link.next
-            link_prev.next = link_next
-            link_next.prev = link_prev
-            link.prev = None
-            link.next = None
-            return result
-        if default is marker:
-            raise KeyError(key)
-        return default
+        if default is self.__marker:
+            return super().pop(key)
+        else:
+            return super().pop(key, default)
 
     def setdefault(self, key, default=None):
         '''Insert key with a value of default if key is not in the dictionary.
 
         Return the value for key if key is in the dictionary, else default.
         '''
-        if key in self:
-            return self[key]
-        self[key] = default
-        return default
+        return super().setdefault(key, default)
 
     @_recursive_repr()
     def __repr__(self):

Lib/test/test_ordered_dict.py

@@ -453,13 +453,6 @@ def test_move_to_end_issue25406(self):
         od.move_to_end('c')
         self.assertEqual(list(od), list('bac'))
 
-    def test_sizeof(self):
-        OrderedDict = self.OrderedDict
-        # Wimpy test: Just verify the reported size is larger than a regular dict
-        d = dict(a=1)
-        od = OrderedDict(**d)
-        self.assertGreater(sys.getsizeof(od), sys.getsizeof(d))
-
     def test_views(self):
         OrderedDict = self.OrderedDict
         # See http://bugs.python.org/issue24286
@@ -557,14 +550,22 @@ def __hash__(self):
             od[key] = i
 
         # These should not crash.
-        with self.assertRaises(KeyError):
+        try:
             list(od.values())
-        with self.assertRaises(KeyError):
+        except KeyError:
+            pass
+        try:
             list(od.items())
-        with self.assertRaises(KeyError):
+        except KeyError:
+            pass
+        try:
             repr(od)
-        with self.assertRaises(KeyError):
+        except KeyError:
+            pass
+        try:
             od.copy()
+        except KeyError:
+            pass
 
     def test_issue24348(self):
         OrderedDict = self.OrderedDict
@@ -615,8 +616,10 @@ def test_dict_delitem(self):
         od['spam'] = 1
         od['ham'] = 2
         dict.__delitem__(od, 'spam')
-        with self.assertRaises(KeyError):
+        try:
             repr(od)
+        except KeyError:
+            pass
 
     def test_dict_clear(self):
         OrderedDict = self.OrderedDict
@@ -632,17 +635,21 @@ def test_dict_pop(self):
         od['spam'] = 1
         od['ham'] = 2
         dict.pop(od, 'spam')
-        with self.assertRaises(KeyError):
+        try:
             repr(od)
+        except KeyError:
+            pass
 
     def test_dict_popitem(self):
         OrderedDict = self.OrderedDict
         od = OrderedDict()
         od['spam'] = 1
         od['ham'] = 2
         dict.popitem(od)
-        with self.assertRaises(KeyError):
+        try:
             repr(od)
+        except KeyError:
+            pass
 
     def test_dict_setdefault(self):
         OrderedDict = self.OrderedDict