Skip to content

BUG: Fixed nlargest/smallest functionality for dataframes with MultiIndex columns #23034

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 7 commits into from
Oct 24, 2018
Merged

BUG: Fixed nlargest/smallest functionality for dataframes with MultiIndex columns #23034

merged 7 commits into from
Oct 24, 2018

Conversation

KristianHolsheimer
Copy link
Contributor

@KristianHolsheimer KristianHolsheimer commented Oct 8, 2018 •
edited
Loading

This change fixes nlargest/smallest functionality for dataframes with MultiIndex columns.

@pep8speaks
Copy link

Hello @KristianHolsheimer! Thanks for submitting the PR.

@KristianHolsheimer KristianHolsheimer mentioned this pull request Oct 8, 2018
Closed
@jreback
Copy link
Contributor

jreback commented Oct 8, 2018

tests

@codecov
Copy link

codecov bot commented Oct 8, 2018 •
edited
Loading

Codecov Report

Merging #23034 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #23034   +/-   ##
=======================================
  Coverage   92.22%   92.22%           
=======================================
  Files         169      169           
  Lines       50903    50903           
=======================================
  Hits        46946    46946           
  Misses       3957     3957
Flag Coverage Δ
#multiple 90.65% <100%> (ø) ⬆️
#single 42.29% <0%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/algorithms.py 95.11% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a6fb0ab...f77f164. Read the comment docs.

@KristianHolsheimer KristianHolsheimer changed the title By convention, tuples indicate multi-level lookups. BUG: Fixed nlargest/smallest functionality for dataframes with MultiIndex columns Oct 8, 2018
jreback
jreback requested changes Oct 9, 2018
View reviewed changes
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

pls add a whatsnew note (bug fixes, reshaping)

pandas/tests/frame/test_analytics.py Outdated
df = pd.DataFrame(
columns=pd.MultiIndex.from_product([['x'], ['a', 'b']]),
data=[[0.33, 0.13], [0.86, 0.25], [0.25, 0.70], [0.85, 0.91]])
pd.util.testing.assert_frame_equal(
Copy link
Contributor

Choose a reason for hiding this comment

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

just use tm.assert_frame_equal its already imported.
use

result =
expected =
tm.assert_frame_equal(result, expected)

put a comment for each case (if needed, may not be here)

@jreback jreback added Bug Reshaping Concat, Merge/Join, Stack/Unstack, Explode MultiIndex labels Oct 9, 2018
@jreback jreback added this to the 0.24.0 milestone Oct 11, 2018
@jreback
Copy link
Contributor

jreback commented Oct 11, 2018

lgtm. ping on green.

@KristianHolsheimer
Copy link
Contributor Author

@jreback
Hi Jeff,

I'd been away from my PRs for a few days. I fixed a merge conflict in the whatsnew doc, but didn't do anything else. This triggered the tests and some of them are failing now. They fail on stuff I never touched, though (rolling etc.).

Do you know what's going on?

Thanks,
Kris

@KristianHolsheimer
Copy link
Contributor Author

retest this please

@WillAyd
Copy link
Member

WillAyd commented Oct 23, 2018

Can you try merging master?

jreback
jreback requested changes Oct 23, 2018
View reviewed changes
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

small change. ping when pushed.

@KristianHolsheimer
Copy link
Contributor Author

@jreback I pushed my changes a few hours ago. Tests are still pending.

@jreback jreback merged commit 4a11eac into pandas-dev:master Oct 24, 2018
@jreback
Copy link
Contributor

jreback commented Oct 24, 2018

thanks

@KristianHolsheimer
Copy link
Contributor Author

Thanks for reviewing

tm9k1 pushed a commit to tm9k1/pandas that referenced this pull request Nov 19, 2018
@KristianHolsheimer @tm9k1
BUG: Fixed nlargest/smallest functionality for dataframes with MultiI…
e1fdbc0 
…ndex columns (pandas-dev#23034)
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
@KristianHolsheimer @Pingviinituutti
BUG: Fixed nlargest/smallest functionality for dataframes with MultiI…
3282d8d 
…ndex columns (pandas-dev#23034)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback approved these changes

Assignees
No one assigned
Labels
Bug MultiIndex Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Projects
None yet
Milestone
0.24.0
Development

Successfully merging this pull request may close these issues.

Problem with nlargest / nsmallest on dataframes with MultiIndex columns
4 participants
@KristianHolsheimer @pep8speaks @jreback @WillAyd