8000 MAINT: Use PySlice_GetIndicesEx instead of custom reimplementation by njsmith · Pull Request #7215 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content

MAINT: Use PySlice_GetIndicesEx instead of custom reimplementation #7215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 19, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions numpy/core/include/numpy/npy_3kcompat.h
Original file line number Diff line number Diff line change
Expand Up @@ -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, 10000 slicelength)
#endif

/*
* PyString -> PyBytes
*/
Expand Down
137 changes: 25 additions & 112 deletions numpy/core/src/multiarray/iterators.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -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:
Expand Down Expand Up @@ -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 ****/
Expand Down
5 changes: 0 additions & 5 deletions numpy/core/src/multiarray/iterators.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 3 additions & 6 deletions numpy/core/src/multiarray/mapping.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
20 changes: 6 additions & 14 deletions numpy/core/src/multiarray/nditer_pywrap.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
47 changes: 23 additions & 24 deletions numpy/core/tests/test_indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,38 +52,38 @@ def test_slicing_no_floats(self):
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,:])
90AE 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
Expand Down Expand Up @@ -1134,7 +1134,6 @@ def test_bool_as_int_argument(self):
# 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])

Expand Down
0