-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
Fix no raise dup index when using drop with axis=0 #19230
Changes from 3 commits
d0b39ea
f43dbf8
a4618bf
b49f78d
43fe5b0
7ceafa1
5f7e6a2
6c7bcf2
fa4c9fe
adf2283
c02c400
85e0094
556f959
5776bf3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2909,6 +2909,9 @@ def _drop_axis(self, labels, axis, level=None, errors='raise'): | |
else: | ||
indexer = ~axis.isin(labels) | ||
|
||
if all(indexer) and errors == 'raise': | ||
raise ValueError('{} not found in axis'.format(labels)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should line 3770 in pandas.core.indexes.base also be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can u show a user facing example There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if u change that, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It breaks 15 tests that are expecting that scenario to be a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can u show some more on this eg which tests There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Wouldn't be a ton of effort to just update the tests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah let me have a closer look There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great, thanks! |
||
|
||
slicer = [slice(None)] * self.ndim | ||
slicer[self._get_axis_number(axis_name)] = indexer | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -257,3 +257,21 @@ def test_insert_column_bug_4032(self): | |
expected = DataFrame([[1.3, 1, 1.1], [2.3, 2, 2.2]], | ||
columns=['c', 'a', 'b']) | ||
assert_frame_equal(result, expected) | ||
|
||
data = [[1, 2, 3], [1, 2, 3]] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you move to test_axis_select_reindex where the other drop tests are |
||
|
||
@pytest.mark.parametrize('actual', [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we need to test for axis=1 case as well (see the original issue) |
||
DataFrame(data=data, index=['a', 'a']), | ||
DataFrame(data=data, index=['a', 'b']), | ||
DataFrame(data=data, index=['a', 'b']).set_index([0, 1]), | ||
DataFrame(data=data, index=['a', 'a']).set_index([0, 1]) | ||
]) | ||
def test_raise_on_drop_duplicate_index(self, actual): | ||
|
||
# issue 19186 | ||
level = 0 if isinstance(actual.index, MultiIndex) else None | ||
with pytest.raises(ValueError): | ||
actual.drop('c', level=level, axis=0) | ||
expected_no_err = actual.drop('c', axis=0, level=level, | ||
errors='ignore') | ||
assert_frame_equal(expected_no_err, actual) |
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.
`ValueError`
-->``ValueError``
(double backticks)`Index`
-->``Index``
(double backticks)(:issue:`19186`)