BUG: sorting with large float and multiple columns incorrect #14944
Conversation
This file contains 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
jreback
reviewed
Dec 21, 2016
| columns=("int", "float")) | ||
|
|
||
| df2 = df.sort_values(["int", "float"]) | ||
| assert is_sorted(df2.loc[:, "int"]) |
There was a problem hiding this comment.
use assert_frame_equal and construct the expected result.
jreback
changed the title
jreback
added
Bug
Numeric Operations
having the int equivalent of NaT in an int64 column caused wrong
sorting because this special value was considered as "missing
value".
uweschmitt
force-pushed
the
fix-gh-14922
branch
from
06f2bd2
to
1afdbb8
Compare
having the int equivalent of NaT in an int64 column caused wrong
sorting because this special value was considered as "missing
value".
Current coverage is 84.65% (diff: 100%)@@ master #14944 diff @@
==========================================
Files 144 144
Lines 51021 51031 +10
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 43188 43199 +11
+ Misses 7833 7832 -1
Partials 0 0
|
jreback
reviewed
Dec 21, 2016
|
|
||
| from pandas.util.testing import (assert_series_equal, | ||
| assert_frame_equal, | ||
| assertRaisesRegexp) | ||
| assertRaisesRegexp, | ||
| is_sorted) |
There was a problem hiding this comment.
you don't need to import the is_sorted
|
can you add a similar tests which uses datetimes and a NaT as well (just to verify that it doesn't break). further pls check both ascending and descending. |
jreback
reviewed
Dec 21, 2016
| def test_sort_nat_values_in_int_column(self): | ||
|
|
||
| # GH 14922, sorting with large float and multiple columns incorrect | ||
| int_values = (2, int(NaT)) |
There was a problem hiding this comment.
use np.iinfo(np.int64).min instead (and NaT) is already an int
There was a problem hiding this comment.
Thought NaT is implicitly indicating what went wrong...
|
can you add a whatsnew note (0.20.0, bug fixes) |
jreback
reviewed
Dec 22, 2016
|
|
||
| # and now check if NaT is still considered as "na" for datetime64 | ||
| # columns: | ||
| df = DataFrame(dict(int=int_values, float=float_values), |
jreback
reviewed
Dec 22, 2016
| df = DataFrame(dict(datetime=[Timestamp("2016-01-01"), NaT], | ||
| float=float_values), columns=["datetime", "float"]) | ||
|
|
||
| # check if the dtype is datetime64[ns]: |
jreback
reviewed
Dec 22, 2016
| df = DataFrame(dict(datetime=[Timestamp("2016-01-01"), NaT], | ||
| float=float_values), columns=["datetime", "float"]) | ||
|
|
||
| assert df["datetime"].dtypes == np.dtype("datetime64[ns]"),\ |
There was a problem hiding this comment.
this line here is not needed (the assert of datetime)
|
Ok now ? |
|
thanks! |
ShaharBental
pushed a commit
to ShaharBental/pandas
that referenced
this pull request
Dec 26, 2016
closes pandas-dev#14922 Having the `int` equivalent of `NaT` in an `int64` column caused wrong sorting because this special value was considered as "missing value". Author: Uwe <uwe.schmitt@id.ethz.ch> Closes pandas-dev#14944 from uweschmitt/fix-gh-14922 and squashes the following commits: c244438 [Uwe] further cleanup tests 4f28026 [Uwe] fixed typo in whatsnew/v0.20.0.txt 60cca5d [Uwe] add fix of GH14922 to release notes for 0.20.0 04dcbe8 [Uwe] further test cleanup 21e610c [Uwe] extended tests + minor cleanup 358a31e [Uwe] Merge branch 'fix-pandas-devgh-14922' of github.com:uweschmitt/pandas into fix-pandas-devgh-14922 03699c6 [Uwe] Fix GH 14922 1afdbb8 [Uwe] Fix GH 14922
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #14922
Having the
intequivalent ofNaTin anint64column caused wrong sorting because this special value was considered as "missing value".git diff upstream/master | flake8 --diff