fill rebuilt the array as array[:start] + fill + array[end:]. When start came after end those two slices overlapped, duplicating the shared elements and growing the array while writing no value. Normalize start and end into [0, length] and fill in place with an index loop so an empty span is a no-op and the length stays fixed.
3.0 KiB
Fix fill growing the array when start comes after end
Problem
pydash.fill(array, value, start, end) is documented to fill elements "from
start up to, but not including, end" and to mutate the array in place. When
start is greater than the effective end, it instead grows the array and
duplicates elements:
>>> import pydash
>>> pydash.fill([1, 2, 3, 4], "x", 4, 1)
[1, 2, 3, 4, 3, 2, 3] # expected [1, 2, 3, 4] (empty fill range)
>>> pydash.fill([1, 2, 3], "x", -1, -2)
[1, 2, 3, 3] # expected [1, 2, 3]
An empty fill range should leave the array unchanged (this also matches
lodash's _.fill, the API pydash mirrors).
Root cause
src/pydash/arrays.py, in fill:
if end is None:
end = len(array)
else:
end = min(end, len(array))
array[:] = array[:start] + [value] * len(array[start:end]) + array[end:]
The array is rebuilt from three slices: the head array[:start], the filled
middle, and the tail array[end:]. This only works when
0 <= start <= end <= len(array). When start > end, array[:start] and
array[end:] overlap, so the shared elements appear twice and the array grows.
The fill range length is computed from array[start:end], which is empty in
that case, so no value is actually written either.
Fix
Normalize start and end into the [0, length] range (supporting negative
indices as slicing does) and fill in place with an index loop, keeping the
array length fixed:
length = len(array)
if end is None:
end = length
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
return array
When start >= end the loop simply does nothing, leaving the array untouched.
Test
Extended test_fill in tests/test_arrays.py with reversed and negative spans:
(([1, 2, 3, 4], "x", 4, 1), [1, 2, 3, 4]),
(([1, 2, 3, 4, 5], 0, 3, 2), [1, 2, 3, 4, 5]),
(([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]),
(([1, 2, 3], "x", -1, -2), [1, 2, 3]),
test_fill also asserts the passed array is mutated in place, which the fix
preserves.
Gate
python -m pytest tests/test_arrays.py -k test_fill-> 11 passedpython -m pytest tests/ --ignore=tests/pytest_mypy_testing-> 1920 passed
Counterfactual (fix reverted, tests kept): the reversed-span cases fail, e.g.
assert [1, 2, 3, 4, 3, 2, 3] == [1, 2, 3, 4] (the array was grown).
Flags for the account owner
- The
tests/pytest_mypy_testing/suite fails locally withNameError(thereveal_typetyping plugin is not resolvable in a barepip install -e .environment). Pre-existing and unrelated to this change. - Related but not fixed here:
pad_startpads multi-charactercharsusing the suffix of the repeated pattern (e.g.pad_start("abc", 4, "-.")->".abc"), whereaspad_endand lodash use the pattern prefix ("-abc"). Left out to keep this change to a single concern.