Series.value_counts: Preserve original ordering

Ensure that value_counts returns the same ordering of the indices than the input object
when sorting the values no matter if it is ascending or descending.

This fixes pandas-dev#12679.

Author: tomspur

pandas/core/algorithms.py

@@ -25,7 +25,7 @@
     is_sparse, is_timedelta64_dtype, is_unsigned_integer_dtype,
     needs_i8_conversion)
 from pandas.core.dtypes.generic import ABCIndex, ABCIndexClass, ABCSeries
-from pandas.core.dtypes.missing import isna, na_value_for_dtype
+from pandas.core.dtypes.missing import isnull, isna, na_value_for_dtype
 
 from pandas.core import common as com
 
@@ -706,6 +706,14 @@ def value_counts(values, sort=True, ascending=False, normalize=False,
                 keys = Index(keys)
             result = Series(counts, index=keys, name=name)
 
+            # Use same index as the original values
+            if isinstance(keys, Index) and isnull(values).sum() == 0:
+                result = result.reindex(unique(values))
+                if dropna:
+                    result = result.dropna()
+                    if len(counts) > 0:
+                        result = result.astype(type(counts[0]))
+
     if sort:
         result = result.sort_values(ascending=ascending)
 

pandas/core/series.py

@@ -42,6 +42,7 @@
 from pandas.core.indexing import check_bool_indexer, maybe_convert_indices
 from pandas.core.internals import SingleBlockManager
 from pandas.core.internals.construction import sanitize_array
+from pandas.core.sorting import nargsort
 from pandas.core.strings import StringMethods
 from pandas.core.tools.datetimes import to_datetime
 
@@ -2660,16 +2661,6 @@ def sort_values(self, axis=0, ascending=True, inplace=False,
             raise ValueError("This Series is a view of some other array, to "
                              "sort in-place you must create a copy")
 
-        def _try_kind_sort(arr):
-            # easier to ask forgiveness than permission
-            try:
-                # if kind==mergesort, it can fail for object dtype
-                return arr.argsort(kind=kind)
-            except TypeError:
-                # stable sort not available for object dtype
-                # uses the argsort default quicksort
-                return arr.argsort(kind='quicksort')
-
         arr = self._values
         sortedIdx = np.empty(len(self), dtype=np.int32)
 
@@ -2678,8 +2669,6 @@ def _try_kind_sort(arr):
         good = ~bad
         idx = ibase.default_index(len(self))
 
-        argsorted = _try_kind_sort(arr[good])
-
         if is_list_like(ascending):
             if len(ascending) != 1:
                 raise ValueError('Length of ascending (%d) must be 1 '
@@ -2689,8 +2678,8 @@ def _try_kind_sort(arr):
         if not is_bool(ascending):
             raise ValueError('ascending must be boolean')
 
-        if not ascending:
-            argsorted = argsorted[::-1]
+        argsorted = nargsort(arr[good], kind=kind, ascending=ascending,
+                             na_position=na_position)
 
         if na_position == 'last':
             n = good.sum()

pandas/tests/series/test_value_counts.py

@@ -0,0 +1,58 @@
+from pandas import Series
+import pandas.util.testing as tm
+
+
+def test_original_ordering_value_counts():
+    # All items occour exactly once. No matter if sorted or not, the resulting
+    # values should be in the same order.
+    s = Series(list('bacdef'))
+
+    # Garantee the same index if value_counts(sort=False) is used
+    vc = s.value_counts(sort=False, ascending=False)
+    tm.assert_series_equal(Series(vc.index), s)
+
+    vc = s.value_counts(sort=True, ascending=False)
+    tm.assert_series_equal(Series(vc.index), s)
+
+
+def test_original_ordering_value_counts2():
+    # 'a' is there twice. Sorted, it should be there at the top, unsorted it
+    # should stay where it is.
+    s = Series(list('bacaef'))
+    ref_nonsorted = Series(list('bacef'))
+    ref_sorted = Series(list('abcef'))
+
+    # Garantee the same index if value_counts(sort=False) is used
+    vc = s.value_counts(sort=False, ascending=False)
+    tm.assert_series_equal(Series(vc.index), ref_nonsorted)
+
+    vc = s.value_counts(sort=True, ascending=False)
+    tm.assert_series_equal(Series(vc.index), ref_sorted)
+
+
+def test_original_ordering_value_counts_ascending():
+    # All items occour exactly once. No matter if sorted or not, the resulting
+    # values should be in the same order.
+    s = Series(list('bacdef'))
+
+    # Garantee the same index if value_counts(sort=False) is used
+    vc = s.value_counts(sort=False, ascending=True)
+    tm.assert_series_equal(Series(vc.index), s)
+
+    vc = s.value_counts(sort=True, ascending=True)
+    tm.assert_series_equal(Series(vc.index), s)
+
+
+def test_original_ordering_value_counts_ascending2():
+    # 'a' is there twice. Unsorted, it should be there at the bottom, unsorted
+    # it should stay where it is.
+    s = Series(list('bacaef'))
+    ref_nonsorted = Series(list('bacef'))
+    ref_sorted = Series(list('bcefa'))
+
+    # Garantee the same index if value_counts(sort=False) is used
+    vc = s.value_counts(sort=False, ascending=True)
+    tm.assert_series_equal(Series(vc.index), ref_nonsorted)
+
+    vc = s.value_counts(sort=True, ascending=True)
+    tm.assert_series_equal(Series(vc.index), ref_sorted)