On Mon, Jun 4, 2012 at 10:00 PM, Thouis (Ray) Jones <tho...@gmail.com> wrote: > On Mon, Jun 4, 2012 at 4:27 PM, Thouis (Ray) Jones <tho...@gmail.com> wrote: >> I could look into this. There are only ~10 places the code generates >> this error, so it should be a pretty minor change. > > My initial estimate was low, but not overly so. An initial pass at > adding index/dimension information to IndexErrors is here: > https://github.com/thouis/numpy/tree/index_error_info
Fabulous! I made a few comments there, but also: > A typical result: > >>>> numpy.zeros(3)[5] > Traceback (most recent call last): > File "<stdin>", line 1, in <module> > IndexError: index 5 out of bounds in dimension 0 I would say "for", not "in". "index 5" is a bit ambiguous too... people might mis-read it as the dimension, like, "the 5th index value I gave"? Not sure how to make it unambiguous. Maybe: "IndexError: dimension 0 index out of bounds: got 5, size is 3" ? > I thought it best to have erroring indices report their initial value: > >>>> numpy.zeros(3)[-15] > Traceback (most recent call last): > File "<stdin>", line 1, in <module> > IndexError: index -15 out of bounds in dimension 0 > > This is different from some places in the code where IndexErrors > already had bad index and dimension information (including the maximum > value possible for an index in that dimension). I left these alone, > though most of them would report that the bad index was -12 instead of > -15. For instance: > https://github.com/thouis/numpy/blob/index_error_info/numpy/core/src/multiarray/mapping.c#L1640 I think this code you link to is actually correct, but yeah, it should definitely report whatever the user passed in, or it will be a debugging hindrance rather than a debugging help! > Also there were a few indexing errors that were throwing ValueErrors. > I changed these to IndexErrors. > > If someone could give this a cursory review before I issue a PR, I'd > appreciate it. I don't expect that most of these code paths are > heavily exercised in the tests (but I could be wrong). Perhaps the easiest thing would be to just add a test? It should be about 1 line each per code path... or 2 if you check both the negative and positive versions. def test_index_bound_checking(): assert_raises(IndexError, my_array.__getitem__, (0, 100)) assert_raises(IndexError, my_array.__getitem__, (0, -101)) # etc. - N _______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
