ENH: making value_counts stable/keeping original ordering #39009

realead · 2021-01-06T21:07:00Z

tests added / passed
Ensure all linting tests pass, see here for how to run them
whatsnew entry

The order of the returned keys for value_counts aren't arbitrary (i.e. depending on the used hash function) but are original ordering (when sorted this applies for the keys with the same number of values).

realead · 2021-01-06T21:29:46Z

There are still some issues that needs to be addressed/discussed:

mode needs probably to reuse the same "stable" functionality. Right now there are differences between value_count and mode (see BUG: Inconsistent results of value_counts and mode for different nan-values #39007). If these differences are not deliberate and just a bug/oversight - that would make the life a little bit easier.

value_count has now a special postprocessing step for nans, ensuring that e.g. pd.NA and np.nan are handled as the same object (

pandas/pandas/core/algorithms.py

Lines 877 to 881 in f9ce9d6

    
           mask = isna(values) 
        
           if not dropna and mask.any(): 
        
               if not isna(keys).any(): 
        
                   keys = np.insert(keys, 0, np.NaN) 
        
                   counts = np.insert(counts, 0, mask.sum())

) The downside of this approach is, that nans aren't in the "original order". Probably a better approach would be something like this:

cpdef stable_value_count_object(ndarray[object] values, bint dropna, na_representant=np.nan):
    ...
    for i in range(n):
        val = values[i]
        if checknull(val):
            val = na_representant  # uses one representant for all null-objects
        if not checknull(val) or not dropna:
            k = kh_get_{{ttype}}(table, <PyObject*>val)
            ...

this would be slightly faster than the current approach, would work also for mode out of the box if reused without additional postprocessing, and nans would keep their "original" place.

non-object branch is no longer nogil (71edcca#diff-f4f1cb812ad02cac95da4070745323e8cf31dfbcc65da52744bb6721439210bbR123) Not sure it is a big issue. However, to improve it, probably Int64Vector & Co:

pandas/pandas/_libs/hashtable_class_helper.pxi.in

Lines 197 to 204 in f1d4026

    
           cdef class {{name}}Vector: 
        
               {{if dtype != 'int64'}} 
        
               cdef: 
        
                   bint external_view_exists 
        
                   {{name}}VectorData *data 
        
                   ndarray ao 
        
               {{endif}}

should be improved. Right now there are some issues (e.g. the whole external_view_exists-business is very annoying. Better would be no longer hold the reference to the from to_array returned object, as it cannot be safely used anyway (it could be resized outside making data a dangling pointer)) which I would like not to address in this PR.

realead · 2021-01-06T21:31:05Z

surprising asv_bench showed no changes when ran with:

asv continuous -f 1.05 upstream/master HEAD  -b series_methods -b period -b categoricals -b frame_methods

jreback

looks good a few comments. pls add a whatsnew note in bug fixes reshaping section. also if this is closing multiple issues (iir also the 32-bit ordering one?) then indicate them there as well.

pandas/_libs/hashtable_func_helper.pxi.in

jreback · 2021-01-09T22:22:09Z

pandas/_libs/hashtable_func_helper.pxi.in

+    result_keys = {{dtype.title()}}Vector()
    {{else}}
-    build_count_table_{{dtype}}(values, table, dropna)
+    result_keys = {{'U'+dtype[1::].title()}}Vector()


what does this do?

That was just too clever, changed to {{name} here: 9a17c13

pandas/tests/libs/test_hashtable.py

pandas/_libs/hashtable_func_helper.pxi.in

realead · 2021-01-14T22:02:10Z

I went on with 2. from comment #39009 (comment)

I think 1. and 3. from this comment are better as separate PRs.

jreback · 2021-01-21T19:08:01Z

can you merge master.

…cases

jreback · 2021-01-21T23:49:57Z

https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=52863&view=logs&j=acc1347f-8235-55b0-95c7-0e2189e61659&t=486f34f1-eda6-516a-fc5d-d8195128dacd i think you need to update this test (was added since you did this PR originally)

realead · 2021-01-22T07:32:25Z

@jreback Thanks, fixed now

jreback

lgtm minor comments. ping on green

jreback · 2021-01-22T14:09:28Z

doc/source/whatsnew/v1.3.0.rst

 - Bug in :func:`join` over :class:`MultiIndex` returned wrong result, when one of both indexes had only one level (:issue:`36909`)
 - :meth:`merge_asof` raises ``ValueError`` instead of cryptic ``TypeError`` in case of non-numerical merge columns (:issue:`29130`)
 - Bug in :meth:`DataFrame.join` not assigning values correctly when having :class:`MultiIndex` where at least one dimension is from dtype ``Categorical`` with non-alphabetically sorted categories (:issue:`38502`)
+- ``value_counts`` returns keys in original order (:issue:`12679`)


:meth:`Series.value_counts`, can you add the other issue that is being closed here as well

Done d85b1aa

jreback · 2021-01-22T14:10:56Z

pandas/_libs/hashtable_func_helper.pxi.in

+        is_null = checknull(val)
+        if is_null:
+            val = navalue
+        if not is_null or not dropna:


could be elif

I don't see how it could work with elif (but I'm always struggeling with logic). I have rearanged the if and hope it is clearer now: 230215f

realead · 2021-01-22T20:18:50Z

@jreback green

jreback · 2021-01-22T21:42:38Z

thanks @realead

realead mentioned this pull request Jan 6, 2021

COMPAT: different orderings in value_counts on 32-bit platforms #11227

Closed

jreback added the Algos Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff label Jan 7, 2021

realead force-pushed the stable_count_values branch 2 times, most recently from 6aa9652 to 8718976 Compare January 8, 2021 22:35

jreback requested changes Jan 9, 2021

View reviewed changes

realead force-pushed the stable_count_values branch from 8718976 to 0966c2b Compare January 14, 2021 21:43

realead changed the title ~~WIP: ENH: making value_counts stable/keeping original ordering~~ ENH: making value_counts stable/keeping original ordering Jan 14, 2021

realead force-pushed the stable_count_values branch from 0966c2b to e64b121 Compare January 14, 2021 21:53

realead force-pushed the stable_count_values branch from e64b121 to e532854 Compare January 15, 2021 20:31

realead added 11 commits January 21, 2021 21:03

introducing stable value_counts function

18050cb

cleaning up: remove no longer needed functions

bfa7e57

the order of keys is stable now (the same as in the input), fix test …

773774b

…cases

adding a test testing explicitly for stability

d96118f

fixing docstring tests

ca68f30

keep original order also for null-objects for dtype=np.object

7fd9153

fixing test cases where the position of NaN has changed

9962c3a

introduce name rather than creating XXXVector-names

679044b

adding explanation

8097e5b

adding issue-id to comment

95f2247

adding whatsnew

d480b7c

realead force-pushed the stable_count_values branch from e532854 to d480b7c Compare January 21, 2021 20:20

realead mentioned this pull request Jan 21, 2021

Complex Dtype Support for Hashmap Algos #36482

Merged

4 tasks

fixing a newly created test

90d0c52

jreback added this to the 1.3 milestone Jan 22, 2021

jreback requested changes Jan 22, 2021

View reviewed changes

realead added 2 commits January 22, 2021 15:56

improve whatsnew

d85b1aa

reorder ifs

230215f

jreback approved these changes Jan 22, 2021

View reviewed changes

jreback merged commit 7e531e3 into pandas-dev:master Jan 22, 2021

realead mentioned this pull request Jan 23, 2021

ENH: making mode stable/keeping original ordering #39353

Merged

4 tasks

realead deleted the stable_count_values branch January 27, 2021 19:05

Uh oh!

ENH: making value_counts stable/keeping original ordering #39009

ENH: making value_counts stable/keeping original ordering #39009

Uh oh!

Conversation

realead commented Jan 6, 2021 • edited by jreback Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

realead commented Jan 6, 2021

Uh oh!

realead commented Jan 6, 2021

Uh oh!

jreback left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jreback Jan 9, 2021

Choose a reason for hiding this comment

Uh oh!

realead Jan 14, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

realead commented Jan 14, 2021

Uh oh!

jreback commented Jan 21, 2021

Uh oh!

jreback commented Jan 21, 2021

Uh oh!

realead commented Jan 22, 2021

Uh oh!

jreback left a comment

Choose a reason for hiding this comment

Uh oh!

jreback Jan 22, 2021

Choose a reason for hiding this comment

Uh oh!

realead Jan 22, 2021

Choose a reason for hiding this comment

Uh oh!

jreback Jan 22, 2021

Choose a reason for hiding this comment

Uh oh!

realead Jan 22, 2021

Choose a reason for hiding this comment

Uh oh!

realead commented Jan 22, 2021

Uh oh!

jreback commented Jan 22, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

realead commented Jan 6, 2021 •

edited by jreback

Loading