classification
Title: hi param in bisect module should not accept negative values
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.10
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: rhettinger, samuel72
Priority: normal Keywords:

Created on 2020-05-31 07:16 by samuel72, last changed 2020-07-09 18:18 by rhettinger. This issue is now closed.

Messages (4)
msg370416 - (view) Author: Vikash Raja Samuel Selvin (samuel72) Date: 2020-05-31 07:16
>>> bisect.bisect_right(l, 5.1, -1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: lo must be non-negative

>>> l
[0, 1, 2, 3, 4, 5, 5, 5, 6, 7, 8, 9]

>>> bisect.bisect_right(l, 5.1, 0, -2)
0

In order to be consistent with the behavior for lo and not return  wrong answers when hi is provided with a negative value
msg370421 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-05-31 09:56
I'm -0 on this one.

Originally, there were no range checks at all.  This kept the code small and fast which was important because bisect() is a building block sometimes used in tight loops, random.choices() for example.

Later, a test for negative lo values was added when it was shown that that case sometimes arose in practice.  At that time, a check for negative hi value was skipped because it either didn't seem to arise in practice or that it would occur in conjunction with a negative lo value.

Another issue is that the C implementation already uses a default of -1 when hi=None.  So adding another check, one that we likely don't really need, would be more invasive and complicated than it seems at first.
msg370532 - (view) Author: Vikash Raja Samuel Selvin (samuel72) Date: 2020-06-01 05:21
Thanks for your comments. Just to make sure I understood correctly, even though something like  bisect.bisect_right(l, 5.1, 0, -2) [This returns 0 which is wrong] is allowed, since it is not common / intended it is ok to not check for negative values for hi.
msg373416 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-07-09 18:18
If the implementation made it easy, I would add the check.  But since it would be an invasive change, I'm inclined to pass on it because it is so uncommon — the lo and hi arguments are rarely used, even more rarely with a negative hi paired with a non-negative lo  — I've never seen this arise in practice ever.

If someone does find a real-world case where this was problematic, feel free to reopen this and we'll find a way to squeeze in the extra range check.
History
Date User Action Args
2020-07-09 18:18:43rhettingersetstatus: open -> closed
resolution: wont fix
messages: + msg373416

stage: resolved
2020-06-01 05:21:27samuel72setmessages: + msg370532
2020-05-31 09:56:38rhettingersetmessages: + msg370421
2020-05-31 09:02:54rhettingersetassignee: rhettinger
versions: - Python 3.7, Python 3.8, Python 3.9
2020-05-31 07:26:45SilentGhostsetnosy: + rhettinger

versions: + Python 3.7, Python 3.8, Python 3.9, Python 3.10
2020-05-31 07:16:01samuel72create