-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
CLN: remove compat.lrange #26396
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CLN: remove compat.lrange #26396
Conversation
Codecov Report
@@ Coverage Diff @@
## master #26396 +/- ##
==========================================
- Coverage 91.69% 91.68% -0.01%
==========================================
Files 174 174
Lines 50749 50743 -6
==========================================
- Hits 46534 46524 -10
- Misses 4215 4219 +4
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #26396 +/- ##
==========================================
- Coverage 91.69% 91.68% -0.02%
==========================================
Files 174 174
Lines 50749 50743 -6
==========================================
- Hits 46534 46523 -11
- Misses 4215 4220 +5
Continue to review full report at Codecov.
|
ed33b66
to
78d7cb6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor question but not necessarily a block. I think this looks good - nice work on all of this @topper-123
assert_take_ok(mgr, ax, []) | ||
assert_take_ok(mgr, ax, [0, 0, 0]) | ||
assert_take_ok(mgr, ax, lrange(mgr.shape[ax])) | ||
assert_take_ok(mgr, ax, indexer=[]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was the reason for adding the keyword here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was just to aid readability, I found the meaning of those lists became easier to understand. Though I see now it hasnt been done in all the function calls.
thanks @topper-123 |
Fourth and final part of the removal of
compat.lrange
from the code base.