-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: restore limit in RangeIndex.get_indexer #28671
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
BUG: restore limit in RangeIndex.get_indexer #28671
Conversation
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.
Thanks @charlesdong1991 lgtm. couple of suggestions.
thanks @simonjayhawkins updated reflecting reviews! |
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.
Thanks @charlesdong1991 for the updates. a few questions.
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.
Thanks @charlesdong1991 i think a few more changes and that's it!
Good morning @simonjayhawkins ! and thanks for your reviews! All changed based on your reviews! Have a great day! ^^ |
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.
pandas/tests/indexes/test_range.py
Outdated
@@ -1037,3 +1045,19 @@ def test_engineless_lookup(self): | |||
assert idx.get_loc("a") == -1 | |||
|
|||
assert "_engine" in idx._cache | |||
|
|||
def test_reindex_limit(self): | |||
# GH 28631 |
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.
this doesn’t belong here
rather with other dataframe reindex tests
look in tests/frame
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.
okay, move to tests/frame/test_indexing
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.
looks good overall, just a couple small nitpicks
@charlesdong1991 thanks a lot for the PR! |
@meeseeksdev backport 0.25.x |
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff