Benedikt's requested changes

Author: StanFromIreland

Doc/library/heapq.rst

@@ -16,8 +16,10 @@
 This module provides an implementation of the heap queue algorithm, also known
 as the priority queue algorithm.
 
-Heaps are binary trees for which every parent node has a value less than or
-equal to any of its children.  We refer to this condition as the heap invariant.
+in-heaps (resp. max-heaps) are binary trees for which every parent node
+has a value less than (resp. greater than) or equal to any of its children.
+We refer to this condition as the heap invariant. Unless stated otherwise,
+*heaps* refer to min-heaps.
 
 This implementation uses arrays for which
 ``heap[k] <= heap[2*k+1]`` and ``heap[k] <= heap[2*k+2]`` for all *k*, counting
@@ -82,45 +84,45 @@ The following functions are provided:
    on the heap.
 
 
-For max heaps, the reverse of a heap, the following functions are provided:
+For max-heaps, the reverse of a heap, the following functions are provided:
 
 
 .. function:: heapify_max(x)
 
-   Transform list *x* into a max heap, in-place, in linear time.
+   Transform list *x* into a max-heap, in-place, in linear time.
 
    .. versionadded:: next
 
 
 .. function:: heappush_max(heap, item)
 
-   Push the value *item* onto the max *heap*, maintaining the heap invariant.
+   Push the value *item* onto the max-*heap*, maintaining the heap invariant.
 
    .. versionadded:: next
 
 
 .. function:: heappop_max(heap)
 
-   Pop and return the largest item from the max *heap*, maintaining the heap
-   invariant.  If the max heap is empty, :exc:`IndexError` is raised.  To access the
+   Pop and return the largest item from the max-*heap*, maintaining the heap
+   invariant.  If the max-heap is empty, :exc:`IndexError` is raised.  To access the
    largest item without popping it, use ``heap[0]``.
 
    .. versionadded:: next
 
 
 .. function:: heappushpop_max(heap, item)
 
-   Push *item* on the max heap, then pop and return the largest item from the max
-   *heap*.  The combined action runs more efficiently than :func:`heappush_max`
+   Push *item* on the max-heap, then pop and return the largest item from *heap*.
+   The combined action runs more efficiently than :func:`heappush_max`
    followed by a separate call to :func:`heappop_max`.
 
    .. versionadded:: next
 
 
 .. function:: heapreplace_max(heap, item)
 
-   Pop and return the largest item from the max *heap*, and also push the new *item*.
-   The max heap size doesn't change. If the max heap is empty, :exc:`IndexError` is raised.
+   Pop and return the largest item from the max-heap *heap* and also push the new *item*.
+   The max-heap size doesn't change. If the max-heap is empty, :exc:`IndexError` is raised.
 
    This one step operation is more efficient than a :func:`heappop_max` followed by
    :func:`heappush_max` and can be more appropriate when using a fixed-size heap.

Lib/heapq.py

@@ -213,13 +213,6 @@ def heapify_max(x):
     for i in reversed(range(n//2)):
         _siftup_max(x, i)
 
-# For backwards compadibility
-_heappop_max  = heappop_max
-_heapreplace_max = heapreplace_max
-_heappush_max = heappush_max
-_heappushpop_max = heappushpop_max
-_heapify_max = heapify_max
-
 
 # 'heap' is a heap at all indices >= startpos, except possibly for pos.  pos
 # is the index of a leaf with a possibly out-of-order value.  Restore the
@@ -604,6 +597,12 @@ def nlargest(n, iterable, key=None):
 except ImportError:
     pass
 
+# For backwards compatibility
+_heappop_max  = heappop_max
+_heapreplace_max = heapreplace_max
+_heappush_max = heappush_max
+_heappushpop_max = heappushpop_max
+_heapify_max = heapify_max
 
 if __name__ == "__main__":
 

Lib/test/test_heapq.py

@@ -14,7 +14,7 @@
 # _heapq.nlargest/nsmallest are saved in heapq._nlargest/_smallest when
 # _heapq is imported, so check them there
 func_names = ['heapify', 'heappop', 'heappush', 'heappushpop', 'heapreplace',
-              'heappop_max', 'heapreplace_max', 'heapify_max', 'heappushpop_max',]
+              'heappop_max', 'heapreplace_max', 'heapify_max', 'heappushpop_max']
 
 class TestModules(TestCase):
     def test_py_functions(self):
@@ -96,10 +96,10 @@ def test_max_push_pop(self):
         # 2) Check that the invariant holds for a sorted array
         self.check_max_invariant(results)
 
-        self.assertRaises(TypeError, self.module.heappush, [])
+        self.assertRaises(TypeError, self.module.heappush_max, [])
         try:
-            self.assertRaises(TypeError, self.module.heappush, None, None)
-            self.assertRaises(TypeError, self.module.heappop, None)
+            self.assertRaises(TypeError, self.module.heappush_max, None, None)
+            self.assertRaises(TypeError, self.module.heappop_max, None)
         except AttributeError:
             pass
 
@@ -113,7 +113,7 @@ def check_invariant(self, heap):
     def check_max_invariant(self, heap):
         for pos in range(1, len(heap)):
             parentpos = (pos - 1) >> 1
-            self.assertTrue(heap[parentpos] >= heap[pos])
+            self.assertGreaterEqual(heap[parentpos], heap[pos])
 
     def test_heapify(self):
         for size in list(range(30)) + [20000]:
@@ -129,7 +129,7 @@ def test_heapify_max(self):
             self.module.heapify_max(heap)
             self.check_max_invariant(heap)
 
-        self.assertRaises(TypeError, self.module.heapify, None)
+        self.assertRaises(TypeError, self.module.heapify_max, None)
 
     def test_naive_nbest(self):
         data = [random.randrange(2000) for i in range(1000)]
@@ -198,21 +198,21 @@ def test_heappushpop(self):
     def test_heappushpop_max(self):
         h = []
         x = self.module.heappushpop_max(h, 10)
-        self.assertEqual((h, x), ([], 10))
+        self.assertTupleEqual((h, x), ([], 10))
 
         h = [10]
         x = self.module.heappushpop_max(h, 10.0)
-        self.assertEqual((h, x), ([10], 10.0))
-        self.assertEqual(type(h[0]), int)
-        self.assertEqual(type(x), float)
+        self.assertTupleEqual((h, x), ([10], 10.0))
+        self.assertIsInstance(h[0], int)
+        self.assertIsInstance(x, float)
 
         h = [10]
         x = self.module.heappushpop_max(h, 11)
-        self.assertEqual((h, x), ([11], 10))
+        self.assertTupleEqual((h, x), ([11], 10))
 
         h = [10]
         x = self.module.heappushpop_max(h, 9)
-        self.assertEqual((h, x), ([10], 9))
+        self.assertTupleEqual((h, x), ([10], 9))
 
     def test_heappop_max(self):
         # heapop_max has an optimization for one-item lists which isn't

Modules/_heapqmodule.c

@@ -549,9 +549,11 @@ _heapq_heappushpop_max_impl(PyObject *module, PyObject *heap, PyObject *item)
 {
     PyObject *returnitem;
     int cmp;
+
     if (PyList_GET_SIZE(heap) == 0) {
         return Py_NewRef(item);
     }
+
     PyObject *top = PyList_GET_ITEM(heap, 0);
     Py_INCREF(top);
     cmp = PyObject_RichCompareBool(top, item, Py_LT);
@@ -562,14 +564,19 @@ _heapq_heappushpop_max_impl(PyObject *module, PyObject *heap, PyObject *item)
     if (cmp == 0) {
         return Py_NewRef(item);
     }
+
+    if (PyList_GET_SIZE(heap) == 0) {
+        PyErr_SetString(PyExc_IndexError, "index out of range");
+        return NULL;
+    }
+
     returnitem = PyList_GET_ITEM(heap, 0);
     PyList_SET_ITEM(heap, 0, Py_NewRef(item));
-
-    if (siftup_max((PyListObject *)heap, 0)) {
+    if (siftup_max((PyListObject *)heap, 0) < 0) {
         Py_DECREF(returnitem);
         return NULL;
     }
-    return Py_NewRef(returnitem);
+    return returnitem;
 }
 
 static PyMethodDef heapq_methods[] = {