Skip to content

Index error info #296

Merged
merged 4 commits into from Jun 15, 2012

2 participants

@thouis
thouis commented Jun 6, 2012

Adds a check_and_adjust_index function in common.c that is used to verify indices and adjust them for negative indexing. Invalid indexes produce an IndexError, and the function returns -1. Otherwise, the index is adjusted in-place (for negative indices), and the function returns 0.

The IndexError produced by this function includes information about the invalid value, the array axis where it caused a problem, and the valid range (when this information is available).

Nearly all such indexing adjustment and validation in the numpy source have been replaced by calls to this function, and a few cases of ValueError in indexing have been replaced by IndexError for consistency.

A couple of open questions:

  • In arraytypes.c.src:...fasttake(), there's no way to know the axis in the call to check_and_adjust_index(), so it can't report the axis in the case of an invalid index. Adding an argument to fasttake would allow it to be reported. I wasn't sure this was an acceptable change.

  • In cases of operations that implicitly operate on a flattened array, the IndexError reports that bad indices are out of bound on axis=0, which might be confusing in some cases. One solution would be to add an argument to check_and_adjust_index() indicating whether the array being indexed was 1-dimensional, and not report an axis in this case.

thouis added some commits Jun 4, 2012
@thouis thouis ENH: report bad value and dimenion to IndexError exceptions 4c68a33
@thouis thouis Add check_and_adjust_index(), and replace most index checks with it.
This commit adds a check_and_adjust_index(npy_intp *index, npy_intp max, int axis) function
which checks index against max, setting an IndexError and returning -1 if it's not valid,
and otherwise adjusting index in-place to handle Python's negative indexing, and returning 0.

It also changes most places in the code where indexes were being checked and adjusted with a
call to this function.
56f8659
@thouis thouis Add coverage tests for IndexErrors, fix one bug, clean up two checks
Adds numpy/core/tests/test_indexerrors.py with tests to cover failure cases in indexing not covered
by other tests.

Added a missing check for invalid index in multiarray/iterators.c:iter_ass_sub_int().
Used the new checking code in multiarray/iterators.c:iter_ass_subscript().
Changed a ValueError to an IndexError in multiarray/mapping.c:PyArray_MapIterBind().
330468f
@thouis thouis commented on the diff Jun 7, 2012
numpy/core/src/multiarray/common.c
@@ -506,6 +506,47 @@
return descr;
}
+NPY_NO_EXPORT int
+check_and_adjust_index(npy_intp *index, npy_intp max_item, int axis)
+{
+ /* Check that index is valid, taking into account negative indices */
+ if ((*index < -max_item) || (*index >= max_item)) {
+ /* Try to be as clear as possible about what went wrong. */
+ if (axis >= 0) {
@thouis
thouis added a note Jun 7, 2012

This should be a check against a constant, like NPY_CHECK_UNKNOWN_AXIS (#def'd to -1). That might also allow some future extensions like NPY_CHECK_FLATTENED_AXIS, to allow more informative reports in the case of indexing an implicitly flattened array.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@njsmith
NumPy member
njsmith commented Jun 14, 2012

I was going to tweak the error messages as per mailing list discussion and then go ahead and merge this, but something is horribly broken on Python 2.4 (only).

>>> np.arange(3)[0]
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
IndexError: index 91358524231122944 is out of bounds for axis 0 with size 3

This is on 64-bit Linux. Some sort of 32-bit/64-bit sign extension problem maybe?

@thouis
@thouis
thouis commented Jun 15, 2012

Fixed. I was able to reproduce it on my Mac. I don't think the signature of array_item_nice() should be changed, as it's exported to Python.

@njsmith
NumPy member
njsmith commented Jun 15, 2012

Right, that would do it...

@njsmith njsmith merged commit a83e212 into numpy:master Jun 15, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.