Upstream dgilland/pydash state of the touched files before 829d1ba14c
This commit is contained in:
1 parent
442a17b572
commit
c1a7f624a6
3 files changed
+5
-92
No files matched your search
@@ -1,74 +0,0 @@
|
||||
# Fix `sorted_last_index_of` raising IndexError on empty array
|
||||
|
||||
## Problem
|
||||
|
||||
`pydash.sorted_last_index_of(array, value)` raises `IndexError` when `array` is
|
||||
empty, instead of returning `-1` (the documented "not found" result):
|
||||
|
||||
```python
|
||||
>>> import pydash
|
||||
>>> pydash.sorted_last_index_of([], 10)
|
||||
Traceback (most recent call last):
|
||||
...
|
||||
IndexError: list index out of range
|
||||
```
|
||||
|
||||
The sibling function `sorted_index_of` returns `-1` for the same input, so the
|
||||
two are inconsistent.
|
||||
|
||||
## Root cause
|
||||
|
||||
`src/pydash/arrays.py`, in `sorted_last_index_of`:
|
||||
|
||||
```python
|
||||
index = sorted_last_index(array, value) - 1
|
||||
|
||||
if index < len(array) and array[index] == value:
|
||||
return index
|
||||
else:
|
||||
return -1
|
||||
```
|
||||
|
||||
`sorted_last_index` returns `0` for an empty array (and whenever `value` is
|
||||
smaller than every element), so `index` becomes `-1`. The guard only checks the
|
||||
upper bound (`index < len(array)`); since `-1 < 0` is `True` for an empty array,
|
||||
`array[index]` is evaluated as `array[-1]` on an empty list, raising
|
||||
`IndexError`. For a non-empty array the same `index == -1` accidentally avoids a
|
||||
wrong result only because `array[-1]` happens not to equal a value smaller than
|
||||
`array[0]`; the missing lower-bound check is still a latent correctness issue.
|
||||
|
||||
## Fix
|
||||
|
||||
Add the lower-bound check so a negative computed index is treated as
|
||||
"not found":
|
||||
|
||||
```python
|
||||
if 0 <= index < len(array) and array[index] == value:
|
||||
return index
|
||||
else:
|
||||
return -1
|
||||
```
|
||||
|
||||
## Test
|
||||
|
||||
Extended `test_sorted_last_index_of` in `tests/test_arrays.py` with the
|
||||
empty-array case and a value-below-range case:
|
||||
|
||||
```python
|
||||
([], 10, -1),
|
||||
([2, 3, 4], 1, -1),
|
||||
```
|
||||
|
||||
## Gate
|
||||
|
||||
- `python -m pytest tests/test_arrays.py -k test_sorted_last_index_of` -> 4 passed
|
||||
- `python -m pytest tests/ --ignore=tests/pytest_mypy_testing` -> 1917 passed
|
||||
|
||||
Counterfactual (fix reverted, test kept): the empty-array case fails with
|
||||
`IndexError: list index out of range` at `arrays.py` in `sorted_last_index_of`.
|
||||
|
||||
## Flags for the account owner
|
||||
|
||||
The `tests/pytest_mypy_testing/` suite fails locally with `NameError` (the
|
||||
`reveal_type` typing plugin is not resolvable in a bare `pip install -e .`
|
||||
environment). These failures are pre-existing and unrelated to this change.
|
||||
+5
-10
@@ -531,18 +531,13 @@ def fill(
|
||||
|
||||
.. versionadded:: 3.1.0
|
||||
"""
|
||||
length = len(array)
|
||||
|
||||
if end is None:
|
||||
end = length
|
||||
end = len(array)
|
||||
else:
|
||||
end = min(end, len(array))
|
||||
|
||||
# Normalize to ``[0, length]``, negatives as slicing does, and fill in
|
||||
# place so the length stays fixed.
|
||||
start = max(length + start, 0) if start < 0 else min(start, length)
|
||||
end = max(length + end, 0) if end < 0 else min(end, length)
|
||||
|
||||
for index in range(start, end):
|
||||
array[index] = value # type: ignore
|
||||
# Use this style of assignment so that `array` is mutated.
|
||||
array[:] = array[:start] + [value] * len(array[start:end]) + array[end:] # type: ignore
|
||||
return array # type: ignore
|
||||
|
||||
|
||||
|
||||
@@ -158,14 +158,6 @@ def test_duplicates(case, expected):
|
||||
(([1, 2, 3, 4, 5], 0, 0, 5), [0, 0, 0, 0, 0]),
|
||||
(([1, 2, 3, 4, 5], 0, 0, 8), [0, 0, 0, 0, 0]),
|
||||
(([1, 2, 3, 4, 5], 0, 0, -1), [0, 0, 0, 0, 5]),
|
||||
# start after end must be a no-op and must not grow the array.
|
||||
(([1, 2, 3, 4], "x", 4, 1), [1, 2, 3, 4]),
|
||||
(([1, 2, 3, 4, 5], 0, 3, 2), [1, 2, 3, 4, 5]),
|
||||
# negative start supported the same way slicing does.
|
||||
(([1, 2, 3, 4, 5], 0, -2), [1, 2, 3, 0, 0]),
|
||||
(([1, 2, 3, 4, 5], 0, -2, -1), [1, 2, 3, 0, 5]),
|
||||
# reversed negative span is a no-op, not an array-growing overlap.
|
||||
(([1, 2, 3], "x", -1, -2), [1, 2, 3]),
|
||||
],
|
||||
)
|
||||
def test_fill(case, expected):
|
||||
|
||||
Reference in new issue
Block a user