Skip to content

Commit

Permalink
Fix singleton-range __setitem__ edge case (#12075)
Browse files Browse the repository at this point in the history
When trying to set a length-one range with a length-one array, an off-by-one error in `copying.copy_range` meant that the value was discarded. Fix that, and tidy up the semantics of `copy_range` a little while we're here. Closes #12073.

Authors:
  - Lawrence Mitchell (https://github.com/wence-)

Approvers:
  - GALI PREM SAGAR (https://github.com/galipremsagar)

URL: #12075
  • Loading branch information
wence- authored Nov 14, 2022
1 parent 8668752 commit 825f049
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 20 deletions.
41 changes: 21 additions & 20 deletions python/cudf/cudf/_lib/copying.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -132,36 +132,37 @@ def _copy_range(Column input_column,
return Column.from_unique_ptr(move(c_result))


def copy_range(Column input_column,
def copy_range(Column source_column,
Column target_column,
size_type input_begin,
size_type input_end,
size_type source_begin,
size_type source_end,
size_type target_begin,
size_type target_end,
bool inplace):
"""
Copy input_column from input_begin to input_end to
target_column from target_begin to target_end
"""

if abs(target_end - target_begin) <= 1:
return target_column
Copy a contiguous range from a source to a target column
if target_begin < 0:
target_begin = target_begin + target_column.size

if target_end < 0:
target_end = target_end + target_column.size
Notes
-----
Expects the source and target ranges to have been sanitised to be
in-range for the source and target column respectively. For
example via ``slice.indices``.
"""

if target_begin > target_end:
assert (
source_end - source_begin == target_end - target_begin,
"Source and target ranges must be same length"
)
if target_end >= target_begin and inplace:
# FIXME: Are we allowed to do this when inplace=False?
return target_column

if inplace is True:
_copy_range_in_place(input_column, target_column,
input_begin, input_end, target_begin)
if inplace:
_copy_range_in_place(source_column, target_column,
source_begin, source_end, target_begin)
else:
return _copy_range(input_column, target_column,
input_begin, input_end, target_begin)
return _copy_range(source_column, target_column,
source_begin, source_end, target_begin)


def gather(
Expand Down
10 changes: 10 additions & 0 deletions python/cudf/cudf/tests/test_setitem.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,16 @@ def test_series_set_item(psr, arg):
assert_eq(psr, gsr)


def test_series_setitem_singleton_range():
sr = cudf.Series([1, 2, 3], dtype=np.int64)
psr = sr.to_pandas()
value = np.asarray([7], dtype=np.int64)
sr.iloc[:1] = value
psr.iloc[:1] = value
assert_eq(sr, cudf.Series([7, 2, 3], dtype=np.int64))
assert_eq(sr, psr, check_dtype=True)


@pytest.mark.parametrize(
"df",
[
Expand Down

0 comments on commit 825f049

Please sign in to comment.