Skip to content
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

Improve sorting in dataframe editor #3240

Merged
merged 3 commits into from
Jun 21, 2016
Merged

Improve sorting in dataframe editor #3240

merged 3 commits into from
Jun 21, 2016

Conversation

jitseniesen
Copy link
Member

@jitseniesen jitseniesen commented Jun 20, 2016 •
edited by ccordoba12
Loading

Dataframe editor: Make sorting stable; handle order correctly.

Fixes #3010
Fixes #3020

@jitseniesen jitseniesen changed the title Dataframe editor Improve sorting in dataframe editor Jun 20, 2016
@ccordoba12 ccordoba12 added this to the v3.0beta4 milestone Jun 20, 2016
Nodd
Nodd reviewed Jun 20, 2016
View reviewed changes
spyderlib/widgets/variableexplorer/tests/test_dataframeeditor.py
df = DataFrame({'colA': [1, 3], 'colB': ['c', 'a']})
dfm = DataFrameModel(df)
dfm.sort(2)
print(dfm.df)
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫

Copy link
Member

Choose a reason for hiding this comment

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

Yep, please remove it :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, I hadn't spotted that I left the print in. It's gone now.

@Nodd
Copy link
Contributor

Nodd commented Jun 20, 2016 •
edited
Loading

The code looks good to me (minus the debug print). And it even has tests ! @jitseniesen you passed the 10% coverage 😄

@jitseniesen
Copy link
Member Author

The code looks good to me. And it even has tests ! @jitseniesen you passed the 10% coverage 😄

I like tests, because I make many mistakes!

@jitseniesen
Dataframe editor: Make sorting stable; handle order correctly.
b6e996c 
- Set ascending flag correctly, fixes spyder-ide#3020.
- Use mergesort so that sorting is stable, fixes spyder-ide#3010.
- Use .sort_values() because .sort() is deprecated since pandas 0.17.
@jitseniesen
Add some tests for dataframe editor.
e8c4023
@jitseniesen
Tests: Remove print from dataframeeditor test.
dee3d80
@ccordoba12
Copy link
Member

What a nice addition @jitseniesen!! Thanks a lot for helping us with it and for increasing our testing coverage too!

Note: I saw you rebased your PR to fix failures in Travis. Usually those failures are caused by random segfaults or tracebacks, so there's no need to worry about that. I usually fix it by launching new builds before merging :-)

However, if you want to trigger a new build in Travis and AppVeyor yourself, you can easily do it by closing and opening the PR (instead of rebasing :-).

@ccordoba12 ccordoba12 merged commit 854a048 into spyder-ide:master Jun 21, 2016
@goanpeca
Copy link
Member

Awesome :-)

Thanks a lot @jitseniesen

@jitseniesen jitseniesen deleted the dataframe-editor branch June 22, 2016 07:53
@jitseniesen
Copy link
Member Author

However, if you want to trigger a new build in Travis and AppVeyor yourself, you can easily do it by closing and opening the PR (instead of rebasing :-).

Thanks, that's good to know. It is a bit annoying that the builds fail that often.

@goanpeca
Copy link
Member

Thanks, that's good to know. It is a bit annoying that the builds fail that often.

Yes it is!!!, but sometimes the segfaults in tests... "seem" to be random :-|

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.0beta4
Development

Successfully merging this pull request may close these issues.
4 participants
@jitseniesen @Nodd @ccordoba12 @goanpeca