This repository was archived by the owner on Dec 22, 2019. It is now read-only.
forked from pandas-dev/pandas
-
Notifications
You must be signed in to change notification settings - Fork 0
Deprecating sort=None #6
Open
lowerthansound
wants to merge
17
commits into
secondtry
Choose a base branch
from
secondtry-deprecate
base: secondtry
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6930e09
to
7e2259a
Compare
I'm currently organizing all append related tests into this one unique module. The old tests have not been deleted yet because their location is kinda useful (for git).
Slowly deprecating sort=None.
I intend to write some regression tests later, or, totally drop the None behavior if possible.
The behavior of these changed and is already being tested in some new functions
This method is responsible for `DataFrame.loc[i] = value`. It uses append in its inner workings. Though I haven't tested yet, I think that setting sort to False is the right choice here, because, never would we want to sort the DataFrame columns when we are just adding some rows.
Function is called by pivot_table
This are all (or almost all) tests that use the method DataFrame.append and that are not located in concat/append test modules specifically. So, what we have left to do: - pandas/tests/reshape/test_concat.py - pandas/tests/frame/test_combine_concat.py Both have been transferred to pandas/tests/reshape/test_append.py (which branch?)
Called during crosstab
a922889
to
07677b9
Compare
lowerthansound
commented
Sep 30, 2018
@@ -6338,6 +6338,9 @@ def append(self, other, ignore_index=False, | |||
3 3 | |||
4 4 | |||
""" | |||
if sort is not False and sort is not True: |
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.
Remove this before merge
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit searches for all locations where sort=None (in practice, where it is ommited) and adds a sort=BOOL in place, correponding to the expected behavior.
We also change the location of old tests and do some quick testing stuff. A bit messy, but will come out nicely
what is that weird matplotlib error?