-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
BUG: MultiIndex.drop does not raise if labels are partially found #37830
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
Conversation
pandas/core/indexes/multi.py
Outdated
@@ -2157,9 +2157,10 @@ def _drop_from_level(self, codes, level, errors="raise"): | |||
index = self.levels[i] | |||
values = index.get_indexer(codes) | |||
|
|||
not_found = np.array(codes)[np.array(values) == -1].tolist() |
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 won't work after #37794 is merged. Could you rebase afterwards? Please check for len(not_found)
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.
ok
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.
is the to_list necessary?
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.
not necessary, I will alter this after phofl's PR merged.
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.
You could rebase now
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.
will review after rebase
cc @jreback |
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, very minor comment, can you merge master and ping on green.
cc @phofl @jbrockmendel if comments
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.
Thx, minor comments
@@ -147,3 +147,22 @@ def test_drop_with_nan_in_index(nulls_fixture): | |||
msg = r"labels \[Timestamp\('2001-01-01 00:00:00'\)\] not found in level" | |||
with pytest.raises(KeyError, match=msg): | |||
mi.drop(pd.Timestamp("2001"), level="date") | |||
|
|||
|
|||
def test_single_level_drop(): |
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.
Could you rename to something like test_single_level_drop_partially_missing_elements?
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.
@GYHHAHA can you do this one, ping on green.
thanks @GYHHAHA very nice, keep em coming! |
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff