From 825f0491cb07dec70cda20865c0f2e3fd3f984f3 Mon Sep 17 00:00:00 2001 From: Lawrence Mitchell Date: Mon, 14 Nov 2022 10:44:38 +0000 Subject: [PATCH] Fix singleton-range `__setitem__` edge case (#12075) 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: https://github.com/rapidsai/cudf/pull/12075 --- python/cudf/cudf/_lib/copying.pyx | 41 +++++++++++++------------- python/cudf/cudf/tests/test_setitem.py | 10 +++++++ 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/python/cudf/cudf/_lib/copying.pyx b/python/cudf/cudf/_lib/copying.pyx index d9a7a5b8754..1de91e6a3e9 100644 --- a/python/cudf/cudf/_lib/copying.pyx +++ b/python/cudf/cudf/_lib/copying.pyx @@ -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( diff --git a/python/cudf/cudf/tests/test_setitem.py b/python/cudf/cudf/tests/test_setitem.py index ac9dbecda65..0298a62b9d2 100644 --- a/python/cudf/cudf/tests/test_setitem.py +++ b/python/cudf/cudf/tests/test_setitem.py @@ -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", [