From 9a6a4b90d5de5ccd4eaca1cc52c9f3a1143f4095 Mon Sep 17 00:00:00 2001 From: Peter Li Date: Thu, 1 Mar 2018 21:01:30 +0800 Subject: [PATCH 1/8] fix rank issue when asec is false --- pandas/_libs/algos_rank_helper.pxi.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/_libs/algos_rank_helper.pxi.in b/pandas/_libs/algos_rank_helper.pxi.in index 9348d7525c307..a0426dd3ff91e 100644 --- a/pandas/_libs/algos_rank_helper.pxi.in +++ b/pandas/_libs/algos_rank_helper.pxi.in @@ -135,7 +135,7 @@ def rank_1d_{{dtype}}(object in_arr, ties_method='average', ascending=True, sorted_data = values.take(_as) sorted_mask = mask.take(_as) - _indices = order[1].take(_as).nonzero()[0] + _indices = np.diff(sorted_mask).nonzero()[0] non_na_idx = _indices[0] if len(_indices) > 0 else -1 argsorted = _as.astype('i8') From dba2258b326ff513bb7a8a05a10affdec92931e9 Mon Sep 17 00:00:00 2001 From: Peter Li Date: Sun, 4 Mar 2018 21:43:02 +0800 Subject: [PATCH 2/8] fix the non_na_idx --- pandas/_libs/algos_rank_helper.pxi.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pandas/_libs/algos_rank_helper.pxi.in b/pandas/_libs/algos_rank_helper.pxi.in index a0426dd3ff91e..b2551f3733904 100644 --- a/pandas/_libs/algos_rank_helper.pxi.in +++ b/pandas/_libs/algos_rank_helper.pxi.in @@ -153,7 +153,7 @@ def rank_1d_{{dtype}}(object in_arr, ties_method='average', ascending=True, if (i == n - 1 or are_diff(util.get_value_at(sorted_data, i + 1), val) or - i == non_na_idx - 1): + i == non_na_idx): if tiebreak == TIEBREAK_AVERAGE: for j in range(i - dups + 1, i + 1): ranks[argsorted[j]] = sum_ranks / dups @@ -190,7 +190,7 @@ def rank_1d_{{dtype}}(object in_arr, ties_method='average', ascending=True, count += 1.0 if (i == n - 1 or sorted_data[i + 1] != val or - i == non_na_idx - 1): + i == non_na_idx): if tiebreak == TIEBREAK_AVERAGE: for j in range(i - dups + 1, i + 1): ranks[argsorted[j]] = sum_ranks / dups From 5eb57b03b1821bb820f8adebfd3cadc0eee85f3c Mon Sep 17 00:00:00 2001 From: Peter Li Date: Sat, 10 Mar 2018 09:36:30 +0800 Subject: [PATCH 3/8] add test ranking inf/nan in descending order --- pandas/tests/series/test_rank.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/pandas/tests/series/test_rank.py b/pandas/tests/series/test_rank.py index d15325ca8ef0e..e3707afedbc88 100644 --- a/pandas/tests/series/test_rank.py +++ b/pandas/tests/series/test_rank.py @@ -16,6 +16,7 @@ from pandas.tests.series.common import TestData from pandas._libs.tslib import iNaT from pandas._libs.algos import Infinity, NegInfinity +from itertools import chain class TestSeriesRank(TestData): @@ -263,8 +264,9 @@ def test_rank_tie_methods_on_infs_nans(self): chunk = 3 disabled = set([('object', 'first')]) - def _check(s, expected, method='average', na_option='keep'): - result = s.rank(method=method, na_option=na_option) + def _check(s, expected, method='average', na_option='keep', ascending=True): + expected = list(chain.from_iterable(expected)) + result = s.rank(method=method, na_option=na_option, ascending=ascending) tm.assert_series_equal(result, Series(expected, dtype='float64')) exp_ranks = { @@ -283,12 +285,13 @@ def _check(s, expected, method='average', na_option='keep'): if (dtype, method) in disabled: continue if na_opt == 'top': - order = ranks[1] + ranks[0] + ranks[2] + order = [ranks[1], ranks[0], ranks[2]] elif na_opt == 'bottom': - order = ranks[0] + ranks[2] + ranks[1] + order = [ranks[0], ranks[2], ranks[1]] else: - order = ranks[0] + [np.nan] * chunk + ranks[1] - _check(iseries, order, method, na_opt) + order = [ranks[0], [np.nan] * chunk, ranks[1]] + _check(iseries, order, method, na_opt, True) + _check(iseries, order[::-1], method, na_opt, False) def test_rank_methods_series(self): pytest.importorskip('scipy.stats.special') From 162251531bd7d5e8dbd411a47584cc0d225d0589 Mon Sep 17 00:00:00 2001 From: Peter Li Date: Sat, 10 Mar 2018 11:53:58 +0800 Subject: [PATCH 4/8] fix some style errors in test_rank --- pandas/tests/series/test_rank.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pandas/tests/series/test_rank.py b/pandas/tests/series/test_rank.py index e3707afedbc88..1293885ec034a 100644 --- a/pandas/tests/series/test_rank.py +++ b/pandas/tests/series/test_rank.py @@ -264,9 +264,11 @@ def test_rank_tie_methods_on_infs_nans(self): chunk = 3 disabled = set([('object', 'first')]) - def _check(s, expected, method='average', na_option='keep', ascending=True): + def _check(s, expected, method='average', na_option='keep', + ascending=True): expected = list(chain.from_iterable(expected)) - result = s.rank(method=method, na_option=na_option, ascending=ascending) + result = s.rank(method=method, na_option=na_option, + ascending=ascending) tm.assert_series_equal(result, Series(expected, dtype='float64')) exp_ranks = { From f6d0fd04cbcf87ad5cb076a90d834c55e02f63ed Mon Sep 17 00:00:00 2001 From: peterpanmj Date: Mon, 12 Mar 2018 16:53:40 +0800 Subject: [PATCH 5/8] parametrize test_rank_tie_methods_on_infs_nans, add a small test for #19538 --- pandas/tests/series/test_rank.py | 53 +++++++++++++++++++------------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/pandas/tests/series/test_rank.py b/pandas/tests/series/test_rank.py index 1293885ec034a..035593cf5b86c 100644 --- a/pandas/tests/series/test_rank.py +++ b/pandas/tests/series/test_rank.py @@ -17,6 +17,7 @@ from pandas._libs.tslib import iNaT from pandas._libs.algos import Infinity, NegInfinity from itertools import chain +import pandas.util._test_decorators as td class TestSeriesRank(TestData): @@ -258,19 +259,16 @@ def _check(s, expected, method='average'): series = s if dtype is None else s.astype(dtype) _check(series, results[method], method=method) - def test_rank_tie_methods_on_infs_nans(self): + @td.skip_if_no_scipy + @pytest.mark.parametrize('ascending', [True, False]) + @pytest.mark.parametrize('method', ['average', 'min', 'max', 'first', + 'dense']) + @pytest.mark.parametrize('na_option', ['top', 'bottom', 'keep']) + def test_rank_tie_methods_on_infs_nans(self, method, na_option, ascending): dtypes = [('object', None, Infinity(), NegInfinity()), ('float64', np.nan, np.inf, -np.inf)] chunk = 3 disabled = set([('object', 'first')]) - - def _check(s, expected, method='average', na_option='keep', - ascending=True): - expected = list(chain.from_iterable(expected)) - result = s.rank(method=method, na_option=na_option, - ascending=ascending) - tm.assert_series_equal(result, Series(expected, dtype='float64')) - exp_ranks = { 'average': ([2, 2, 2], [5, 5, 5], [8, 8, 8]), 'min': ([1, 1, 1], [4, 4, 4], [7, 7, 7]), @@ -278,22 +276,33 @@ def _check(s, expected, method='average', na_option='keep', 'first': ([1, 2, 3], [4, 5, 6], [7, 8, 9]), 'dense': ([1, 1, 1], [2, 2, 2], [3, 3, 3]) } - na_options = ('top', 'bottom', 'keep') + + def _check(s, method, na_option, ascending): + ranks = exp_ranks[method] + if na_option == 'top': + order = [ranks[1], ranks[0], ranks[2]] + elif na_option == 'bottom': + order = [ranks[0], ranks[2], ranks[1]] + else: + order = [ranks[0], [np.nan] * chunk, ranks[1]] + expected = order if ascending else order[::-1] + expected = list(chain.from_iterable(expected)) + result = s.rank(method=method, na_option=na_option, + ascending=ascending) + tm.assert_series_equal(result, Series(expected, dtype='float64')) + for dtype, na_value, pos_inf, neg_inf in dtypes: in_arr = [neg_inf] * chunk + [na_value] * chunk + [pos_inf] * chunk iseries = Series(in_arr, dtype=dtype) - for method, na_opt in product(exp_ranks.keys(), na_options): - ranks = exp_ranks[method] - if (dtype, method) in disabled: - continue - if na_opt == 'top': - order = [ranks[1], ranks[0], ranks[2]] - elif na_opt == 'bottom': - order = [ranks[0], ranks[2], ranks[1]] - else: - order = [ranks[0], [np.nan] * chunk, ranks[1]] - _check(iseries, order, method, na_opt, True) - _check(iseries, order[::-1], method, na_opt, False) + if (dtype, method) in disabled: + continue + _check(iseries, method, na_option, ascending) + + def test_rank_desc_mix_nans_infs(self): + iseries = Series([1, np.nan, np.inf, -np.inf, 25]) + result = iseries.rank(ascending=False) + exp = Series([3, np.nan, 1, 4, 2], dtype='float64') + tm.assert_series_equal(result, exp) def test_rank_methods_series(self): pytest.importorskip('scipy.stats.special') From 8b0467f62dfc94c6b557e8b49feb12228d135bb1 Mon Sep 17 00:00:00 2001 From: peterpanmj Date: Thu, 15 Mar 2018 10:50:29 +0800 Subject: [PATCH 6/8] add issue number and move expected results into comparing method , add some comments --- pandas/tests/series/test_rank.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/pandas/tests/series/test_rank.py b/pandas/tests/series/test_rank.py index 035593cf5b86c..92ecc08154215 100644 --- a/pandas/tests/series/test_rank.py +++ b/pandas/tests/series/test_rank.py @@ -269,15 +269,15 @@ def test_rank_tie_methods_on_infs_nans(self, method, na_option, ascending): ('float64', np.nan, np.inf, -np.inf)] chunk = 3 disabled = set([('object', 'first')]) - exp_ranks = { - 'average': ([2, 2, 2], [5, 5, 5], [8, 8, 8]), - 'min': ([1, 1, 1], [4, 4, 4], [7, 7, 7]), - 'max': ([3, 3, 3], [6, 6, 6], [9, 9, 9]), - 'first': ([1, 2, 3], [4, 5, 6], [7, 8, 9]), - 'dense': ([1, 1, 1], [2, 2, 2], [3, 3, 3]) - } def _check(s, method, na_option, ascending): + exp_ranks = { + 'average': ([2, 2, 2], [5, 5, 5], [8, 8, 8]), + 'min': ([1, 1, 1], [4, 4, 4], [7, 7, 7]), + 'max': ([3, 3, 3], [6, 6, 6], [9, 9, 9]), + 'first': ([1, 2, 3], [4, 5, 6], [7, 8, 9]), + 'dense': ([1, 1, 1], [2, 2, 2], [3, 3, 3]) + } ranks = exp_ranks[method] if na_option == 'top': order = [ranks[1], ranks[0], ranks[2]] @@ -299,6 +299,8 @@ def _check(s, method, na_option, ascending): _check(iseries, method, na_option, ascending) def test_rank_desc_mix_nans_infs(self): + #GH 19538 + #check descending ranking when mix nans and infs iseries = Series([1, np.nan, np.inf, -np.inf, 25]) result = iseries.rank(ascending=False) exp = Series([3, np.nan, 1, 4, 2], dtype='float64') From b50374322341123f5197c00de3bc0c86d836fe39 Mon Sep 17 00:00:00 2001 From: peterpanmj Date: Mon, 19 Mar 2018 13:04:18 +0800 Subject: [PATCH 7/8] fix some pep8 errors --- pandas/tests/series/test_rank.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pandas/tests/series/test_rank.py b/pandas/tests/series/test_rank.py index 92ecc08154215..004e42e14cb93 100644 --- a/pandas/tests/series/test_rank.py +++ b/pandas/tests/series/test_rank.py @@ -272,11 +272,11 @@ def test_rank_tie_methods_on_infs_nans(self, method, na_option, ascending): def _check(s, method, na_option, ascending): exp_ranks = { - 'average': ([2, 2, 2], [5, 5, 5], [8, 8, 8]), - 'min': ([1, 1, 1], [4, 4, 4], [7, 7, 7]), - 'max': ([3, 3, 3], [6, 6, 6], [9, 9, 9]), - 'first': ([1, 2, 3], [4, 5, 6], [7, 8, 9]), - 'dense': ([1, 1, 1], [2, 2, 2], [3, 3, 3]) + 'average': ([2, 2, 2], [5, 5, 5], [8, 8, 8]), + 'min': ([1, 1, 1], [4, 4, 4], [7, 7, 7]), + 'max': ([3, 3, 3], [6, 6, 6], [9, 9, 9]), + 'first': ([1, 2, 3], [4, 5, 6], [7, 8, 9]), + 'dense': ([1, 1, 1], [2, 2, 2], [3, 3, 3]) } ranks = exp_ranks[method] if na_option == 'top': @@ -299,8 +299,8 @@ def _check(s, method, na_option, ascending): _check(iseries, method, na_option, ascending) def test_rank_desc_mix_nans_infs(self): - #GH 19538 - #check descending ranking when mix nans and infs + # GH 19538 + # check descending ranking when mix nans and infs iseries = Series([1, np.nan, np.inf, -np.inf, 25]) result = iseries.rank(ascending=False) exp = Series([3, np.nan, 1, 4, 2], dtype='float64') From 62e96f69d42f5e2f5a2325500ae7a53091064926 Mon Sep 17 00:00:00 2001 From: Peter Li Date: Sun, 25 Mar 2018 18:11:41 +0800 Subject: [PATCH 8/8] update whatsnew --- doc/source/whatsnew/v0.23.0.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/source/whatsnew/v0.23.0.txt b/doc/source/whatsnew/v0.23.0.txt index 791365295c268..279ab48c20255 100644 --- a/doc/source/whatsnew/v0.23.0.txt +++ b/doc/source/whatsnew/v0.23.0.txt @@ -918,6 +918,7 @@ Numeric - Bug in :class:`DataFrame` flex arithmetic (e.g. ``df.add(other, fill_value=foo)``) with a ``fill_value`` other than ``None`` failed to raise ``NotImplementedError`` in corner cases where either the frame or ``other`` has length zero (:issue:`19522`) - Multiplication and division of numeric-dtyped :class:`Index` objects with timedelta-like scalars returns ``TimedeltaIndex`` instead of raising ``TypeError`` (:issue:`19333`) - Bug where ``NaN`` was returned instead of 0 by :func:`Series.pct_change` and :func:`DataFrame.pct_change` when ``fill_method`` is not ``None`` (provided) (:issue:`19873`) +- Bug in :meth:`Series.rank` and :meth:`DataFrame.rank` when ``ascending='False'`` failed to return correct ranks for infinity if ``NaN`` were present (:issue:`19538`) Indexing