-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: BooleanArray.value_counts dropna #30824
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
4d1abac
b3796a5
604f170
12cfbeb
67c0c06
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -411,6 +411,25 @@ Use :meth:`arrays.IntegerArray.to_numpy` with an explicit ``na_value`` instead. | |
|
||
a.to_numpy(dtype="float", na_value=np.nan) | ||
|
||
**value_counts returns a nullable integer dtype** | ||
|
||
:meth:`Series.value_counts` with a nullable integer dtype now returns a nullable | ||
integer dtype for the values. | ||
|
||
*pandas 0.25.x* | ||
|
||
.. code-block:: python | ||
|
||
>>> pd.Series([2, 1, 1, None], dtype="Int64").value_counts().dtype | ||
dtype('int64') | ||
|
||
*pandas 1.0.0* | ||
|
||
.. ipython:: python | ||
|
||
>>> pd.Series([2, 1, 1, None], dtype="Int64").value_counts().dtype | ||
Int64Dtype() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't show the output (or >>> prompt), or otherwise also make it a code-block There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Whoops, thanks. |
||
|
||
See :ref:`missing_data.NA` for more on the differences between :attr:`pandas.NA` | ||
and :attr:`numpy.nan`. | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -856,3 +856,14 @@ def test_arrow_roundtrip(): | |
result = table.to_pandas() | ||
assert isinstance(result["a"].dtype, pd.BooleanDtype) | ||
tm.assert_frame_equal(result, df) | ||
|
||
|
||
def test_value_counts_na(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is the result an object dtyped Series when dropna=True? (add a test as well) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In neither cases (dropna True of False) is the result an object dtype series, it is always integer (it are just counts). That said, should the result here rather be a nullable integer type? Not that there are nulls here, but in the light of "trying to return nullable types as much as possible from operations involving nullable types". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm, yeah i think we should just move this to return a nullable integer (as this is new api). will promote consistency in the future. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, OK, will update these. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And so API breaking change for IntegerARrray.value_counts to return a nullalble int dtype too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you expand on why you find it weird? It's true that the result of a value_counts will always have no NAs, but returning a nullable int type prevents a reintroduction of NAs in subsequent operations from converting to float. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The motivation is to maintain consistency of "operations with nullable types return nullable types". But making value_counts().values return IntNA breaks the consistency of "values_counts().values is always np.int64". So it's a wash on "maintaining consistency". Ideally we'd retain the dtype in the value_counts().index, and it seems like we're saying here "well we cant do that, so let's shoehorn the dtype into the values" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
No, I don't think we're saying that. I think we're saying we find a nullable integer dtype to be more useful. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a hill I want to die on. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. commented below |
||
arr = pd.array([True, False, pd.NA], dtype="boolean") | ||
result = arr.value_counts(dropna=False) | ||
expected = pd.Series([1, 1, 1], index=[True, False, pd.NA], dtype="Int64") | ||
tm.assert_series_equal(result, expected) | ||
|
||
result = arr.value_counts(dropna=True) | ||
expected = pd.Series([1, 1], index=[True, False], dtype="Int64") | ||
tm.assert_series_equal(result, expected) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, even though is a breaking change, it puts nullable in more general usage. so actually i think this is a good change.