API: ArrowExtensionArray.value_counts returns pyarrow.int64 type #51542
Conversation
| @@ -873,6 +873,9 @@ def value_counts( | |||
| result.name = name | |||
| result.index.name = index_name | |||
| counts = result._values | |||
| if not isinstance(counts, np.ndarray): | |||
| # e.g. ArrowExtensionArray | |||
| counts = np.asarray(counts) | |||
There was a problem hiding this comment.
why is this needed? id expect pd.value_counts to behave like obj.value_counts
There was a problem hiding this comment.
For normalize below, counts.sum() is called and ._values which is an ArrowExtensionArray doesn't have sum.
I think it was assumed that counts is a numpy array here?
There was a problem hiding this comment.
If EA's implement value_counts they should also handle normalize?
There was a problem hiding this comment.
I think it was assumed that counts is a numpy array here?
For a long time I wanted value_counts to have int64 dtype and get rid of Int64 cases, but eventually gave up on that. So i think at least Int64 is possible. IIRC value_counts is only kinda-sorta part of the EA interface so the exact requirements aren't pinned down.
If EA's implement value_counts they should also handle normalize?
If the normalization logic can live exclusively here, I think I'd prefer that.
which is an ArrowExtensionArray doesn't have sum.
Out of scope, but I'd just put sum and the other most-basic reductions (say, all the ones in the np.ndarray namespace) on EA and let it raise TypeError for non-supported dtypes.
There was a problem hiding this comment.
Would you prefer me to put sum on ArrowExtensionArray so this works without the if statement or should we leave in this check?
There was a problem hiding this comment.
i guess cleaner to leave this and can do another api change later if called for
pandas/tests/extension/test_arrow.py
Outdated
|
|
||
| result = ser.value_counts(normalize=True).sort_index() | ||
|
|
||
| if not isinstance(data, pd.Categorical): |
There was a problem hiding this comment.
we can get here with Categorical?
There was a problem hiding this comment.
Good point, yeah we can't. This is a copy/paste from the base class. Removed
|
Greenish |
|
thanks @mroeschke |
…eturns pyarrow.int64 type
…unts returns pyarrow.int64 type) (#51620) Backport PR #51542: API: ArrowExtensionArray.value_counts returns pyarrow.int64 type Co-authored-by: Matthew Roeschke <[email protected]>
* note breaking change in json_normalize retaining index For context: #51542 & #57422 * Update doc/source/whatsnew/v3.0.0.rst * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Matthew Roeschke <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.