-
-
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
BUG: drop_duplicates drops name(s). #10367
Conversation
35f10ab
to
acddfaa
Compare
acddfaa
to
60a6e9f
Compare
self.assertIsNone(idx_dup.freq) # freq is reset | ||
result = idx_dup.drop_duplicates() | ||
self.assert_index_equal(idx, result) | ||
self.assertIsNone(result.freq) |
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.
hmm I think that these tests are in test_index.py
(there is of course some duplication, pardon the pun, with where tests live for these sub-classes).
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.
I intended these tests are for freq
rather than name
, because name
are tested in test_index
. Maybe assert_index_equal
should check all metadata to simplify the test (I've once tried and found some tests / logics must be fixed).
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.
oh, that's fine then.
lgtm. ideally let's try to group tests that work across ALL sub-classes in |
merge when ready |
BUG: drop_duplicates drops name(s).
Thank for review:) |
Closes #10115. Closes #10116.
Based on @seth-p and @jreback 's fix, added tets for datetime-likes.