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.
BUG: stabilize sort_values algorithms for Series and time-like Indices #37310
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: stabilize sort_values algorithms for Series and time-like Indices #37310
Changes from 9 commits
151c425
7332255
546b9fa
12eb535
9b51d42
7805d1e
2b75f78
2844a97
965a547
29d47ee
151196d
d71bfc8
aff28ac
0b8aae9
e7cebc4
06931e0
0b24c3e
1b98bff
4076f0c
08aadd3
6f904e6
5c7eea9
75aad12
e503dca
759de34
12741f2
d433952
dc906df
dbf295e
cbe528e
6658b73
076fa7a
cdf63a3
dd30ec9
5cac5c7
7a68a45
4c72dbf
3599591
3ee9b8b
d63293c
a5c8f65
fc90ea9
408abe0
5f53cfc
4370f27
6099344
bfa2b28
bc004ec
cd7111e
5fbbf7d
95b9f25
1dc9b89
2946e46
488596c
c8cd8cd
ab71697
05f60d5
8669e89
00b454c
5d5f3d0
3d7f47c
351a003
8de6ac9
18bb141
9b97302
3a88ebe
d41789e
812f312
e28ce4d
0719633
61ac60d
d495064
36932cd
2156c64
e6f5741
c823043
cd66748
37a6439
d09da99
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Support everything except
MultiIndex
(see PR header).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.
this comment is not super useful, what is an expl of why you need this try/except e.g. L380 move here
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.
No longer relevant, since I removed the try/except.
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.
why can this fail?
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.
@jreback I moved this code from
Series.sort_values
, fearing that I don't understand why it is necessary, but it doesn't seem to be. I've removed the try/except, tested sortingSeries
ofobject
with differentkind
settings, and ran the test suite, and nothing is going wrong. I think we can safely drop it.