Skip to content

Fix singleton-range __setitem__ edge case - #12075

Merged
rapids-bot[bot] merged 4 commits into
NVIDIA:branch-22.12from
wence-:wence/fix/issue-12073
Nov 14, 2022
Merged

Fix singleton-range __setitem__ edge case#12075
rapids-bot[bot] merged 4 commits into
NVIDIA:branch-22.12from
wence-:wence/fix/issue-12073

Conversation

@wence-

@wence- wence- commented Nov 4, 2022

Copy link
Copy Markdown
Contributor

Description

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.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Only when copying a zero-length range can we afford to return the
target column. Closes NVIDIA#12073.
@wence-
wence- requested a review from a team as a code owner November 4, 2022 18:04
@wence-
wence- requested review from charlesbluca and shwina November 4, 2022 18:04
@github-actions github-actions Bot added the Python Affects Python cuDF API. label Nov 4, 2022
@wence- wence- added 3 - Ready for Review Ready for review by team non-breaking Non-breaking change labels Nov 4, 2022
Comment thread python/cudf/cudf/_lib/copying.pyx Outdated
Precondition is that range indices have been sanitised.
@wence-
wence- force-pushed the wence/fix/issue-12073 branch from 8c87b98 to ca2eba3 Compare November 4, 2022 20:03
Comment thread python/cudf/cudf/tests/test_setitem.py
@codecov

codecov Bot commented Nov 4, 2022

Copy link
Copy Markdown

Codecov Report

Base: 87.47% // Head: 88.11% // Increases project coverage by +0.63% 🎉

Coverage data is based on head (9e9c20a) compared to base (f817d96).
Patch has no changes to coverable lines.

Additional details and impacted files
@@               Coverage Diff                @@
##           branch-22.12   #12075      +/-   ##
================================================
+ Coverage         87.47%   88.11%   +0.63%     
================================================
  Files               133      135       +2     
  Lines             21826    22100     +274     
================================================
+ Hits              19093    19473     +380     
+ Misses             2733     2627     -106     
Impacted Files Coverage Δ
python/cudf/cudf/core/column/interval.py 85.45% <0.00%> (-9.10%) ⬇️
python/cudf/cudf/io/text.py 91.66% <0.00%> (-8.34%) ⬇️
python/cudf/cudf/core/_base_index.py 81.28% <0.00%> (-4.27%) ⬇️
python/cudf/cudf/io/json.py 92.06% <0.00%> (-2.68%) ⬇️
python/cudf/cudf/utils/utils.py 89.91% <0.00%> (-0.69%) ⬇️
python/cudf/cudf/core/column/timedelta.py 90.17% <0.00%> (-0.58%) ⬇️
python/cudf/cudf/core/column/datetime.py 89.21% <0.00%> (-0.51%) ⬇️
python/cudf/cudf/core/column/column.py 87.96% <0.00%> (-0.46%) ⬇️
python/dask_cudf/dask_cudf/core.py 73.72% <0.00%> (-0.41%) ⬇️
python/cudf/cudf/io/parquet.py 90.45% <0.00%> (-0.39%) ⬇️
... and 43 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@wence- wence- added the bug Something isn't working label Nov 7, 2022
Comment thread python/cudf/cudf/_lib/copying.pyx
Comment thread python/cudf/cudf/tests/test_setitem.py
@galipremsagar galipremsagar added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels Nov 14, 2022
@galipremsagar

Copy link
Copy Markdown
Contributor

@gpucibot merge

@rapids-bot
rapids-bot Bot merged commit 825f049 into NVIDIA:branch-22.12 Nov 14, 2022
@wence-
wence- deleted the wence/fix/issue-12073 branch November 14, 2022 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

5 - Ready to Merge Testing and reviews complete, ready to merge bug Something isn't working non-breaking Non-breaking change Python Affects Python cuDF API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Missing data with length-one sliced-based __setitem__ and singleton array

2 participants