This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: bisect index out of bounds issue
Type: enhancement Stage:
Components: Library (Lib) Versions: Python 3.5
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: Paul.Ianas, rhettinger, tim.peters
Priority: normal Keywords:

Created on 2014-10-21 07:23 by Paul.Ianas, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Messages (4)
msg229750 - (view) Author: Paul Ianas (Paul.Ianas) Date: 2014-10-21 07:23
The precondition for all the bisect functions is implemented like this:

    if lo < 0:
        raise ValueError('lo must be non-negative')
    if hi is None:
        hi = len(a)

Now, of course, if hi is given, and hi >= 2 * len(a), then we get an IndexError. In case hi < 0, we always get 0 as a result (even if the element is there).

I think it would be better to treat the hi in the precondition in the same way as the lo parameter: that means, raise a ValueError in case hi has an illegal value.

Disclaimer: of course, it makes no sense to give an illegal argument to that function; still, since lo is treated against illegal values, maybe it's better to do the same for hi.

At the same time, maybe moving the precondition code in a separate function (which raises a ValueError in case precondition is not met) makes more sense, for not repeating the same code in all bisect functions.

A small snippet which reproduces this:

from bisect import bisect_left

a = [1, 2, 3, 4]
idx = bisect_left(a, 2, 0, 10)  # 10 > 2 * 4
print(idx)
msg229855 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2014-10-23 05:16
These functions are very old and the API is unlikely to change, particularly if the goal is to change one kind of exception to another (making bad inputs fail in a different way than they do now).  As far as I can tell, the current arrangement has never been a actual problem in practice.
msg229857 - (view) Author: Paul Ianas (Paul.Ianas) Date: 2014-10-23 06:52
Sure, it is your call. As said, this is rather an enhancement.

Still, if I were to decide, I would chose:
1. not to break the API <=> raise IndexError instead of ValueError in case hi is invalid.
2. to protect against illegal values: as said, if hi < 0 bisect_* always returns 0 (whatever the searched value).
3. I would implement a single _range_check(_len, lo, hi) method to do this logic (DRY).

That being said, from my point of view this ticket can be closed.
msg230390 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2014-10-31 21:22
> treat the hi in the precondition in the same way as the lo parameter

Originally, there was no special test for lo.  The test was added only because negative lo value failed in an unfortunate way (with an infinite loop).  No change to hi was made because (it succeeded in some cases and existing code might be relying on that or it failed in clear-cut way by raising an IndexError).

> That being said, from my point of view this ticket can be closed.

I concur.  When there isn't a clear case for change, it is usually better to favor stability over potential disruption.
History
Date User Action Args
2022-04-11 14:58:09adminsetgithub: 66873
2014-10-31 21:22:48rhettingersetstatus: open -> closed
resolution: rejected
messages: + msg230390
2014-10-23 06:52:37Paul.Ianassetmessages: + msg229857
2014-10-23 05:16:04rhettingersetmessages: + msg229855
versions: - Python 2.7, Python 3.2, Python 3.3, Python 3.4
2014-10-23 05:02:29rhettingersetassignee: rhettinger
2014-10-21 08:23:28pitrousetnosy: + tim.peters, rhettinger
2014-10-21 07:23:12Paul.Ianascreate