Skip to content

TST: Test renaming a multiindex with duplicates (GH 38015) #43187

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Sep 8, 2021
Merged

TST: Test renaming a multiindex with duplicates (GH 38015) #43187

merged 4 commits into from
Sep 8, 2021

Conversation

andrewwoood
Copy link
Contributor

@andrewwoood andrewwoood commented Aug 23, 2021 •
edited
Loading

This is my first real contribution, I appreciate the feedback!

phofl
phofl reviewed Aug 23, 2021
View reviewed changes
pandas/tests/indexing/multiindex/test_multiindex.py Outdated

def test_rename_multiindex_with_duplicates(self):
# GH 38015
idx = pd.Index([("A", "cat"), ("B", "cat"), ("B", "cat")])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in General. Please use the MultiIndex constructor and compare the whole dataframe

@pep8speaks
Copy link

pep8speaks commented Aug 23, 2021 •
edited
Loading

Hello @andrewwoood! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-09-05 05:02:05 UTC

@phofl
Copy link
Member

phofl commented Aug 24, 2021

Some pre-commit issues, otherwise lgtm.

You could install pre-commit to run the hooks locally before pushing

@jreback jreback added MultiIndex Testing pandas testing functions or related to the test suite labels Aug 31, 2021
@jreback jreback added this to the 1.4 milestone Aug 31, 2021
@jreback
Copy link
Contributor

jreback commented Aug 31, 2021

can you merge master and ping on green

@andrewwoood andrewwoood force-pushed the tst-multiindex-rename branch from 362def3 to e95ad00 Compare September 2, 2021 21:04
@andrewwoood
Copy link
Contributor Author

@jreback Sorry, is this what you were asking me to do? I'm not very competent with using git and contributing to open-source quite yet. I tried using the pre-commit framework but it doesn't seem to help my file pass the pre-commit check.

@phofl
Copy link
Member

phofl commented Sep 3, 2021

The ci tells you whats wrong:

 pandas/tests/indexing/multiindex/test_multiindex.py:125:14: PDF019 found both 'pd.MultiIndex' and 'MultiIndex' in the same file
pandas/tests/indexing/multiindex/test_multiindex.py:126:14: PDF019 found both 'pd.DataFrame' and 'DataFrame' in the same file
pandas/tests/indexing/multiindex/test_multiindex.py:129:15: PDF019 found both 'pd.MultiIndex' and 'MultiIndex' in the same file
pandas/tests/indexing/multiindex/test_multiindex.py:130:20: PDF019 found both 'pd.DataFrame' and 'DataFrame' in the same file

@mroeschke mroeschke merged commit 5018d20 into pandas-dev:master Sep 8, 2021
@mroeschke
Copy link
Member

Thanks @andrewwoood! Happy to have you address similar good first issues that need tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phofl phofl phofl left review comments

@mroeschke mroeschke mroeschke approved these changes

Assignees
No one assigned
Labels
MultiIndex Testing pandas testing functions or related to the test suite
Projects
None yet
Milestone
1.4
Development

Successfully merging this pull request may close these issues.

BUG: df.rename raises when multiindex has duplicates
5 participants
@andrewwoood @pep8speaks @phofl @jreback @mroeschke