Fix bug when iloc slice terminates at before-the-zero position
#7277
Conversation
isVoid
added
bug
|
@isVoid can you retarget this to 0.18? This is a nice bugfix with minimal changes that would be good to get in. |
|
[Updated] Moving |
| @@ -831,6 +833,66 @@ def __sizeof__(self): | |||
| index = self._index.__sizeof__() | |||
| return columns + index | |||
|
|
|||
| def _slice(self: T, arg: slice) -> T: | |||
There was a problem hiding this comment.
Does this not get used underneath Series.iloc[slice(...)] as well?
There was a problem hiding this comment.
Series.iloc seems to depend on ColumnBase.__getitem__, which later calls ColumnBase.slice.
cudf/python/cudf/cudf/core/indexing.py
Line 79 in 3ecde9d
I also noticed the following:
>>> s = cudf.Series([1,2,3,4,5])
>>> s.iloc[4:-1:-1]
Series([], dtype: int64)which is likely the same issue, investigating.
There was a problem hiding this comment.
It's the same issue in ColumnBase.slice.
cudf/python/cudf/cudf/core/column/column.py
Lines 609 to 610 in b8cb8c7
Submitted a quick fix. Although I think the best fix here is to consolidate Series.iloc and DataFrame.iloc to share the common code path under Frame._slice.
kkraus14
added
5 - Ready to Merge
kkraus14
dismissed
their stale review
Additional changes based on Series.slice
|
rerun tests |
kkraus14
added
the
5 - Ready to Merge
|
@gpucibot merge |
Codecov Report
@@ Coverage Diff @@
## branch-0.18 #7277 +/- ##
===============================================
+ Coverage 82.09% 82.19% +0.10%
===============================================
Files 97 100 +3
Lines 16474 16954 +480
===============================================
+ Hits 13524 13936 +412
- Misses 2950 3018 +68
Continue to review full report at Codecov.
|
|
rerun tests |
|
@isVoid There seem to be some conflicts, could you resolve them? |
Closes #7246
This PR fixes a bug in
Dataframe.iloc. When the slice provided toiloc, is decrementing and also terminates atbefore-the-zeroposition, such asslice(2, -1, -1)orslice(4, None, -1), the terminal position still gets wrapped around.Frame._sliceis moved toDataFrame._sliceto resolve typing issue.