BUG: nunique not ignoring both None and np.nan #37607
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
arw2019
suggested changes
Nov 3, 2020
pandas/core/base.py
Outdated
| @@ -1032,10 +1032,11 @@ def nunique(self, dropna: bool = True) -> int: | |||
| >>> s.nunique() | |||
| 4 | |||
| """ | |||
| uniqs = self.unique() | |||
| if dropna: | |||
There was a problem hiding this comment.
I'd do
obj = self.dropna() if dropna else self
return len(obj.unique())
pandas/tests/base/test_unique.py
Outdated
|
|
||
|
|
||
| def test_nunique_dropna(): | ||
| # test for #37566 |
pandas/tests/base/test_unique.py
Outdated
| @@ -121,3 +121,11 @@ def test_unique_bad_unicode(idx_or_series_w_bad_unicode): | |||
| else: | |||
| expected = np.array(["\ud83d"], dtype=object) | |||
| tm.assert_numpy_array_equal(result, expected) | |||
|
|
|||
|
|
|||
| def test_nunique_dropna(): | |||
There was a problem hiding this comment.
Parametrize on dropna (True/False)
pandas/tests/base/test_unique.py
Outdated
| def test_nunique_dropna(): | ||
| # test for #37566 | ||
| s = pd.Series(['yes', 'yes', pd.NA, np.nan, None, pd.NaT]) | ||
| res = s.nunique(dropna=True) |
doc/source/whatsnew/v1.2.0.rst
Outdated
| @@ -562,6 +562,7 @@ Other | |||
| - Bug in :meth:`Index.union` behaving differently depending on whether operand is a :class:`Index` or other list-like (:issue:`36384`) | |||
| - Passing an array with 2 or more dimensions to the :class:`Series` constructor now raises the more specific ``ValueError``, from a bare ``Exception`` previously (:issue:`35744`) | |||
| - Bug in ``accessor.DirNamesMixin``, where ``dir(obj)`` wouldn't show attributes defined on the instance (:issue:`37173`). | |||
| - Bug in :meth:`nunique` with ``dropna = True`` returns wrong result when different kinds of NA-like values exist (:issue:`37566`) | |||
There was a problem hiding this comment.
Bug in :meth:`Series.nunique` with ``dropna = True`` was returning incorrect results when both ``NA`` and ``None`` missing values were present (:issue:`37566`)
jreback
added
Bug
Reshaping
arw2019
reviewed
Nov 4, 2020
pandas/tests/base/test_unique.py
Outdated
| def test_nunique_dropna(dropna): | ||
| # GH37566 | ||
| s = pd.Series(["yes", "yes", pd.NA, np.nan, None, pd.NaT]) | ||
| res = s.nunique(dropna=True) |
There was a problem hiding this comment.
res = s.nunique(dropna) here, no?
arw2019
approved these changes
Nov 4, 2020
phofl
reviewed
Nov 10, 2020
doc/source/whatsnew/v1.2.0.rst
Outdated
| @@ -562,6 +562,7 @@ Other | |||
| - Bug in :meth:`Index.union` behaving differently depending on whether operand is a :class:`Index` or other list-like (:issue:`36384`) | |||
| - Passing an array with 2 or more dimensions to the :class:`Series` constructor now raises the more specific ``ValueError``, from a bare ``Exception`` previously (:issue:`35744`) | |||
| - Bug in ``accessor.DirNamesMixin``, where ``dir(obj)`` wouldn't show attributes defined on the instance (:issue:`37173`). | |||
| - Bug in :meth:`Series.nunique` with ``dropna = True`` was returning incorrect results when both ``NA`` and ``None`` missing values were present (:issue:`37566`) | |||
There was a problem hiding this comment.
Could you delete spaces around =?
|
looks good, just @phofl comment i think is left, ping on green. |
|
cc @jreback |
|
thanks @GYHHAHA very nice |
| if dropna and isna(uniqs).any(): | ||
| n -= 1 | ||
| return n | ||
| obj = remove_na_arraylike(self) if dropna else self |
There was a problem hiding this comment.
Late comment, but would it be more performant if we removed the NAs after calculating the uniques? (assume the uniques is typically a much smaller array, and not this does an extra scan and creates a copy of the full array)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff