Add default= kwarg to .list.get() accessor method #10547
Conversation
shwina
added
feature request
Codecov Report
@@ Coverage Diff @@
## branch-22.06 #10547 +/- ##
===============================================
Coverage ? 86.34%
===============================================
Files ? 140
Lines ? 22303
Branches ? 0
===============================================
Hits ? 19257
Misses ? 3046
Partials ? 0 Continue to review full report at Codecov.
|
|
@bdice could you take another look? I realized I had a bug in my implementation - fixed in this commit. |
| if not (default is None or default is cudf.NA): | ||
| # determine rows for which `index` is out-of-bounds | ||
| lengths = count_elements(self._column) | ||
| out_of_bounds_indexes = (-index > lengths) | (index >= lengths) |
There was a problem hiding this comment.
(Non-blocker) Do we have a grammatical rule on indexes vs indices? :)
There was a problem hiding this comment.
(Slightly more of a blocker, on further inspection) These aren't really indexes/indices - this is a boolean column, right? That might deserve a name like out_of_bounds_mask.
There was a problem hiding this comment.
Changed to out_of_bounds_mask.
There was a problem hiding this comment.
The change from using - to np.negative came from cherry-picking commits from the branch #10564 where I inadvertently made changes first locally. We're eventually going to have to keep that change anyway.
Co-authored-by: Bradley Dice <bdice@bradleydice.com>
This reverts commit 2cb1df3.
|
@gpucibot merge |
1 similar comment
|
@gpucibot merge |
…10564) Closes #10552. Depends on #10547 Authors: - Ashwin Srinath (https://github.com/shwina) Approvers: - Bradley Dice (https://github.com/bdice) - Vyas Ramasubramani (https://github.com/vyasr) - Charles Blackmon-Luca (https://github.com/charlesbluca) URL: #10564
Closes #10540.
As mentioned in the issue, this is a breaking change, although we could introduce this change in a non-breaking way by using a sentinel value for the kwarg if desired.