MAINT: Use PySlice_GetIndicesEx instead of custom reimplementation

This has the side effects of:
- changing several IndexError exceptions into TypeErrors
- allowing slices like `arr[False:True]` as equivalent to
  `arr[0:1]` (because now we're using Python's logic for interpreting
  slices, and Python is happy with treating bools as integers in integer
  contexts).

It also deletes almost 100 lines of code :-).

While I was at it I also cleaned up some buggy uses of
PySlice_GetIndices (which is pretty broken -- e.g. the code was assuming
that it sets an exception on error, but this is not true! the Python
docs explicitly recommend that you never use it.)

6 files changed, 65 insertions, 161 deletions

diff --git a/numpy/core/include/numpy/npy_3kcompat.h b/numpy/core/include/numpy/npy_3kcompat.h
index db60a312c..cdab1bbe8 100644
--- a/numpy/core/include/numpy/npy_3kcompat.h
+++ b/numpy/core/include/numpy/npy_3kcompat.h
@@ -53,6 +53,14 @@ static NPY_INLINE int PyInt_Check(PyObject *op) {
  */
 #endif /* NPY_PY3K */
 
+/* Py3 changes PySlice_GetIndicesEx' first argument's type to PyObject* */
+#ifdef NPY_PY3K
+#  define NpySlice_GetIndicesEx PySlice_GetIndicesEx
+#else
+#  define NpySlice_GetIndicesEx(op, nop, start, end, step, slicelength) \
+    PySlice_GetIndicesEx((PySliceObject *)op, nop, start, end, step, slicelength)
+#endif
+
 /*
  * PyString -> PyBytes
  */
diff --git a/numpy/core/src/multiarray/iterators.c b/numpy/core/src/multiarray/iterators.c
index 5099e3e19..3dd7b0ebb 100644
--- a/numpy/core/src/multiarray/iterators.c
+++ b/numpy/core/src/multiarray/iterators.c
@@ -20,8 +20,22 @@
 #define ELLIPSIS_INDEX -2
 #define SINGLE_INDEX -3
 
+/*
+ * Tries to convert 'o' into an npy_intp interpreted as an
+ * index. Returns 1 if it was successful, 0 otherwise. Does
+ * not set an exception.
+ */
 static int
-slice_coerce_index(PyObject *o, npy_intp *v);
+coerce_index(PyObject *o, npy_intp *v)
+{
+    *v = PyArray_PyIntAsIntp(o);
+
+    if ((*v) == -1 && PyErr_Occurred()) {
+        PyErr_Clear();
+        return 0;
+    }
+    return 1;
+}
 
 /*
  * This function converts one element of the indexing tuple
@@ -46,12 +60,7 @@ parse_index_entry(PyObject *op, npy_intp *step_size,
     }
     else if (PySlice_Check(op)) {
         npy_intp stop;
-        if (slice_GetIndices((PySliceObject *)op, max,
-                             &i, &stop, step_size, n_steps) < 0) {
-            if (!PyErr_Occurred()) {
-                PyErr_SetString(PyExc_IndexError,
-                                "invalid slice");
-            }
+        if (NpySlice_GetIndicesEx(op, max, &i, &stop, step_size, n_steps) < 0) {
             goto fail;
         }
         if (*n_steps <= 0) {
@@ -60,22 +69,22 @@ parse_index_entry(PyObject *op, npy_intp *step_size,
             i = 0;
         }
     }
-    else {
-        if (!slice_coerce_index(op, &i)) {
-            PyErr_SetString(PyExc_IndexError,
-                            "each index entry must be either a "
-                            "slice, an integer, Ellipsis, or "
-                            "newaxis");
-            goto fail;
-        }
+    else if (coerce_index(op, &i)) {
         *n_steps = SINGLE_INDEX;
         *step_size = 0;
         if (check_index) {
             if (check_and_adjust_index(&i, max, axis, NULL) < 0) {
-            goto fail;
+                goto fail;
             }
         }
     }
+    else {
+        PyErr_SetString(PyExc_IndexError,
+                        "each index entry must be either a "
+                        "slice, an integer, Ellipsis, or "
+                        "newaxis");
+        goto fail;
+    }
     return i;
 
  fail:
@@ -190,102 +199,6 @@ parse_index(PyArrayObject *self, PyObject *op,
     return nd_new;
 }
 
-/*
- * Tries to convert 'o' into an npy_intp interpreted as an
- * index. Returns 1 if it was successful, 0 otherwise. Does
- * not set an exception.
- */
-static int
-slice_coerce_index(PyObject *o, npy_intp *v)
-{
-    *v = PyArray_PyIntAsIntp(o);
-
-    if ((*v) == -1 && PyErr_Occurred()) {
-        PyErr_Clear();
-        return 0;
-    }
-    return 1;
-}
-
-
-/*
- * This is basically PySlice_GetIndicesEx, but with our coercion
- * of indices to integers (plus, that function is new in Python 2.3)
- *
- * N.B. The coercion to integers is deprecated; once the DeprecationWarning
- * is changed to an error, it would seem that this is obsolete.
- */
-NPY_NO_EXPORT int
-slice_GetIndices(PySliceObject *r, npy_intp length,
-                 npy_intp *start, npy_intp *stop, npy_intp *step,
-                 npy_intp *slicelength)
-{
-    npy_intp defstop;
-
-    if (r->step == Py_None) {
-        *step = 1;
-    }
-    else {
-        if (!slice_coerce_index(r->step, step)) {
-            return -1;
-        }
-        if (*step == 0) {
-            PyErr_SetString(PyExc_ValueError,
-                            "slice step cannot be zero");
-            return -1;
-        }
-    }
-    /* defstart = *step < 0 ? length - 1 : 0; */
-    defstop = *step < 0 ? -1 : length;
-    if (r->start == Py_None) {
-        *start = *step < 0 ? length-1 : 0;
-    }
-    else {
-        if (!slice_coerce_index(r->start, start)) {
-            return -1;
-        }
-        if (*start < 0) {
-            *start += length;
-        }
-        if (*start < 0) {
-            *start = (*step < 0) ? -1 : 0;
-        }
-        if (*start >= length) {
-            *start = (*step < 0) ? length - 1 : length;
-        }
-    }
-
-    if (r->stop == Py_None) {
-        *stop = defstop;
-    }
-    else {
-        if (!slice_coerce_index(r->stop, stop)) {
-            return -1;
-        }
-        if (*stop < 0) {
-            *stop += length;
-        }
-        if (*stop < 0) {
-            *stop = -1;
-        }
-        if (*stop > length) {
-            *stop = length;
-        }
-    }
-
-    if ((*step < 0 && *stop >= *start) ||
-        (*step > 0 && *start >= *stop)) {
-        *slicelength = 0;
-    }
-    else if (*step < 0) {
-        *slicelength = (*stop - *start + 1) / (*step) + 1;
-    }
-    else {
-        *slicelength = (*stop - *start - 1) / (*step) + 1;
-    }
-
-    return 0;
-}
 
 /*********************** Element-wise Array Iterator ***********************/
 /*  Aided by Peter J. Verveer's  nd_image package and numpy's arraymap  ****/
diff --git a/numpy/core/src/multiarray/iterators.h b/numpy/core/src/multiarray/iterators.h
index dad345935..04f57c885 100644
--- a/numpy/core/src/multiarray/iterators.h
+++ b/numpy/core/src/multiarray/iterators.h
@@ -18,9 +18,4 @@ NPY_NO_EXPORT PyObject
 NPY_NO_EXPORT int
 iter_ass_subscript(PyArrayIterObject *, PyObject *, PyObject *);
 
-NPY_NO_EXPORT int
-slice_GetIndices(PySliceObject *r, npy_intp length,
-                 npy_intp *start, npy_intp *stop, npy_intp *step,
-                 npy_intp *slicelength);
-
 #endif
diff --git a/numpy/core/src/multiarray/mapping.c b/numpy/core/src/multiarray/mapping.c
index 6c56d77bb..cd0d65c95 100644
--- a/numpy/core/src/multiarray/mapping.c
+++ b/numpy/core/src/multiarray/mapping.c
@@ -803,12 +803,9 @@ get_view_from_index(PyArrayObject *self, PyArrayObject **view,
                 }
                 break;
             case HAS_SLICE:
-                if (slice_GetIndices((PySliceObject *)indices[i].object,
-                                     PyArray_DIMS(self)[orig_dim],
-                                     &start, &stop, &step, &n_steps) < 0) {
-                    if (!PyErr_Occurred()) {
-                        PyErr_SetString(PyExc_IndexError, "invalid slice");
-                    }
+                if (NpySlice_GetIndicesEx(indices[i].object,
+                                          PyArray_DIMS(self)[orig_dim],
+                                          &start, &stop, &step, &n_steps) < 0) {
                     return -1;
                 }
                 if (n_steps <= 0) {
diff --git a/numpy/core/src/multiarray/nditer_pywrap.c b/numpy/core/src/multiarray/nditer_pywrap.c
index 67f5ab99f..c735e7ad1 100644
--- a/numpy/core/src/multiarray/nditer_pywrap.c
+++ b/numpy/core/src/multiarray/nditer_pywrap.c
@@ -2227,14 +2227,6 @@ npyiter_seq_ass_slice(NewNpyArrayIterObject *self, Py_ssize_t ilow,
     return 0;
 }
 
-/* Py3 changes PySlice_GetIndices' first argument's type to PyObject* */
-#ifdef NPY_PY3K
-#  define slice_getindices PySlice_GetIndices
-#else
-#  define slice_getindices(op, nop, start, end, step) \
-        PySlice_GetIndices((PySliceObject *)op, nop, start, end, step)
-#endif
-
 static PyObject *
 npyiter_subscript(NewNpyArrayIterObject *self, PyObject *op)
 {
@@ -2260,9 +2252,9 @@ npyiter_subscript(NewNpyArrayIterObject *self, PyObject *op)
         return npyiter_seq_item(self, i);
     }
     else if (PySlice_Check(op)) {
-        Py_ssize_t istart = 0, iend = 0, istep = 0;
-        if (slice_getindices(op, NpyIter_GetNOp(self->iter),
-                            &istart, &iend, &istep) < 0) {
+        Py_ssize_t istart = 0, iend = 0, istep = 0, islicelength;
+        if (NpySlice_GetIndicesEx(op, NpyIter_GetNOp(self->iter),
+                                  &istart, &iend, &istep, &islicelength) < 0) {
             return NULL;
         }
         if (istep != 1) {
@@ -2309,9 +2301,9 @@ npyiter_ass_subscript(NewNpyArrayIterObject *self, PyObject *op,
         return npyiter_seq_ass_item(self, i, value);
     }
     else if (PySlice_Check(op)) {
-        Py_ssize_t istart = 0, iend = 0, istep = 0;
-        if (slice_getindices(op, NpyIter_GetNOp(self->iter),
-                            &istart, &iend, &istep) < 0) {
+        Py_ssize_t istart = 0, iend = 0, istep = 0, islicelength = 0;
+        if (NpySlice_GetIndicesEx(op, NpyIter_GetNOp(self->iter),
+                                  &istart, &iend, &istep, &islicelength) < 0) {
             return -1;
         }
         if (istep != 1) {
diff --git a/numpy/core/tests/test_indexing.py b/numpy/core/tests/test_indexing.py
index deb2130b7..8d6f6a96b 100644
--- a/numpy/core/tests/test_indexing.py
+++ b/numpy/core/tests/test_indexing.py
@@ -52,38 +52,38 @@ class TestIndexing(TestCase):
         a = np.array([[5]])
 
         # start as float.
-        assert_raises(IndexError, lambda: a[0.0:])
-        assert_raises(IndexError, lambda: a[0:, 0.0:2])
-        assert_raises(IndexError, lambda: a[0.0::2, :0])
-        assert_raises(IndexError, lambda: a[0.0:1:2,:])
-        assert_raises(IndexError, lambda: a[:, 0.0:])
+        assert_raises(TypeError, lambda: a[0.0:])
+        assert_raises(TypeError, lambda: a[0:, 0.0:2])
+        assert_raises(TypeError, lambda: a[0.0::2, :0])
+        assert_raises(TypeError, lambda: a[0.0:1:2,:])
+        assert_raises(TypeError, lambda: a[:, 0.0:])
         # stop as float.
-        assert_raises(IndexError, lambda: a[:0.0])
-        assert_raises(IndexError, lambda: a[:0, 1:2.0])
-        assert_raises(IndexError, lambda: a[:0.0:2, :0])
-        assert_raises(IndexError, lambda: a[:0.0,:])
-        assert_raises(IndexError, lambda: a[:, 0:4.0:2])
+        assert_raises(TypeError, lambda: a[:0.0])
+        assert_raises(TypeError, lambda: a[:0, 1:2.0])
+        assert_raises(TypeError, lambda: a[:0.0:2, :0])
+        assert_raises(TypeError, lambda: a[:0.0,:])
+        assert_raises(TypeError, lambda: a[:, 0:4.0:2])
         # step as float.
-        assert_raises(IndexError, lambda: a[::1.0])
-        assert_raises(IndexError, lambda: a[0:, :2:2.0])
-        assert_raises(IndexError, lambda: a[1::4.0, :0])
-        assert_raises(IndexError, lambda: a[::5.0,:])
-        assert_raises(IndexError, lambda: a[:, 0:4:2.0])
+        assert_raises(TypeError, lambda: a[::1.0])
+        assert_raises(TypeError, lambda: a[0:, :2:2.0])
+        assert_raises(TypeError, lambda: a[1::4.0, :0])
+        assert_raises(TypeError, lambda: a[::5.0,:])
+        assert_raises(TypeError, lambda: a[:, 0:4:2.0])
         # mixed.
-        assert_raises(IndexError, lambda: a[1.0:2:2.0])
-        assert_raises(IndexError, lambda: a[1.0::2.0])
-        assert_raises(IndexError, lambda: a[0:, :2.0:2.0])
-        assert_raises(IndexError, lambda: a[1.0:1:4.0, :0])
-        assert_raises(IndexError, lambda: a[1.0:5.0:5.0,:])
-        assert_raises(IndexError, lambda: a[:, 0.4:4.0:2.0])
+        assert_raises(TypeError, lambda: a[1.0:2:2.0])
+        assert_raises(TypeError, lambda: a[1.0::2.0])
+        assert_raises(TypeError, lambda: a[0:, :2.0:2.0])
+        assert_raises(TypeError, lambda: a[1.0:1:4.0, :0])
+        assert_raises(TypeError, lambda: a[1.0:5.0:5.0,:])
+        assert_raises(TypeError, lambda: a[:, 0.4:4.0:2.0])
         # should still get the DeprecationWarning if step = 0.
-        assert_raises(IndexError, lambda: a[::0.0])
+        assert_raises(TypeError, lambda: a[::0.0])
 
     def test_index_no_array_to_index(self):
         # No non-scalar arrays.
         a = np.array([[[1]]])
 
-        assert_raises(IndexError, lambda: a[a:a:a])
+        assert_raises(TypeError, lambda: a[a:a:a])
 
     def test_none_index(self):
         # `None` index adds newaxis
@@ -1134,7 +1134,6 @@ class TestBooleanArgumentErrors(TestCase):
         # array is thus also deprecated, but not with the same message:
         assert_raises(TypeError, operator.index, np.array(True))
         assert_raises(TypeError, np.take, args=(a, [0], False))
-        assert_raises(IndexError, lambda: a[False:True:True])
         assert_raises(IndexError, lambda: a[False, 0])
         assert_raises(IndexError, lambda: a[False, 0, 0])