Skip to content
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

Fix nunique for MultiIndex, DataFrame, and all NA case with dropna=False #15962

Merged
merged 17 commits into from
Jun 14, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion cpp/src/stream_compaction/distinct_count.cu
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,15 @@ cudf::size_type distinct_count(column_view const& input,
nan_policy nan_handling,
rmm::cuda_stream_view stream)
{
if (0 == input.size() or input.null_count() == input.size()) { return 0; }
if (0 == input.size()) { return 0; }

if (input.null_count() == input.size()) {
if (null_handling == null_policy::INCLUDE) {
return 1;
} else {
return 0;
}
mroeschke marked this conversation as resolved.
Show resolved Hide resolved
}
mroeschke marked this conversation as resolved.
Show resolved Hide resolved

auto count = detail::distinct_count(table_view{{input}}, null_equality::EQUAL, stream);

Expand Down
8 changes: 5 additions & 3 deletions python/cudf/cudf/core/dataframe.py
Original file line number Diff line number Diff line change
Expand Up @@ -7475,7 +7475,7 @@ def __dataframe__(
self, nan_as_null=nan_as_null, allow_copy=allow_copy
)

def nunique(self, axis=0, dropna=True):
def nunique(self, axis=0, dropna: bool = True) -> Series:
"""
Count number of distinct elements in specified axis.
Return Series with number of distinct elements. Can ignore NaN values.
Expand Down Expand Up @@ -7503,8 +7503,10 @@ def nunique(self, axis=0, dropna=True):
"""
if axis != 0:
raise NotImplementedError("axis parameter is not supported yet.")

return cudf.Series(super().nunique(dropna=dropna))
counts = [col.distinct_count(dropna=dropna) for col in self._columns]
return self._constructor_sliced(
counts, index=self._data.to_pandas_index()
)

def _sample_axis_1(
self,
Expand Down
7 changes: 3 additions & 4 deletions python/cudf/cudf/core/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -1940,10 +1940,9 @@ def nunique(self, dropna: bool = True):
dict
Name and unique value counts of each column in frame.
"""
return {
name: col.distinct_count(dropna=dropna)
for name, col in self._data.items()
}
raise NotImplementedError(
f"{type(self).__name__} does not implement nunique"
)

@staticmethod
@_cudf_nvtx_annotate
Expand Down
2 changes: 1 addition & 1 deletion python/cudf/cudf/core/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -895,7 +895,7 @@ def __array__(self, dtype=None):
)

@_cudf_nvtx_annotate
def nunique(self) -> int:
def nunique(self, dropna: bool = True) -> int:
return len(self)

@_cudf_nvtx_annotate
Expand Down
8 changes: 8 additions & 0 deletions python/cudf/cudf/core/multiindex.py
Original file line number Diff line number Diff line change
Expand Up @@ -1701,6 +1701,14 @@ def fillna(self, value):
def unique(self):
return self.drop_duplicates(keep="first")

@_cudf_nvtx_annotate
def nunique(self, dropna: bool = True) -> int:
if dropna:
mi = self.dropna(how="all")
else:
mi = self
mroeschke marked this conversation as resolved.
Show resolved Hide resolved
return len(mi.unique())

def _clean_nulls_from_index(self):
"""
Convert all na values(if any) in MultiIndex object
Expand Down
2 changes: 0 additions & 2 deletions python/cudf/cudf/core/single_column_frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -335,8 +335,6 @@ def nunique(self, dropna: bool = True) -> int:
int
Number of unique values in the column.
"""
if self._column.null_count == len(self):
return 0
return self._column.distinct_count(dropna=dropna)

def _get_elements_from_column(self, arg) -> Union[ScalarLike, ColumnBase]:
Expand Down
14 changes: 14 additions & 0 deletions python/cudf/cudf/tests/test_dataframe.py
Original file line number Diff line number Diff line change
Expand Up @@ -9966,6 +9966,20 @@ def test_dataframe_nunique(data):
assert_eq(expected, actual)


@pytest.mark.parametrize(
"columns",
[
pd.RangeIndex(2, name="foo"),
pd.MultiIndex.from_arrays([[1, 2], [2, 3]], names=["foo", 1]),
pd.Index([3, 5], dtype=np.int8, name="foo"),
],
)
def test_nunique_preserve_column_in_index(columns):
df = cudf.DataFrame([[1, 2]], columns=columns)
result = df.nunique().index.to_pandas()
pd.testing.assert_index_equal(result, columns, exact=True)
mroeschke marked this conversation as resolved.
Show resolved Hide resolved


@pytest.mark.parametrize(
"data",
[{"key": [0, 1, 1, 0, 0, 1], "val": [1, 8, 3, 9, -3, 8]}],
Expand Down
11 changes: 11 additions & 0 deletions python/cudf/cudf/tests/test_multiindex.py
Original file line number Diff line number Diff line change
Expand Up @@ -2162,3 +2162,14 @@ def test_multi_index_contains_hashable():
lfunc_args_and_kwargs=((),),
rfunc_args_and_kwargs=((),),
)


@pytest.mark.parametrize("array", [[1, 2], [1, np.nan], [np.nan] * 2])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same concern as below

@pytest.mark.parametrize("dropna", [True, False])
def test_nunique(array, dropna):
arrays = [array, [3, 4]]
gidx = cudf.MultiIndex.from_arrays(arrays)
pidx = pd.MultiIndex.from_arrays(arrays)
result = gidx.nunique(dropna=dropna)
expected = pidx.nunique(dropna=dropna)
assert result == expected
10 changes: 10 additions & 0 deletions python/cudf/cudf/tests/test_series.py
Original file line number Diff line number Diff line change
Expand Up @@ -2848,3 +2848,13 @@ def test_nans_to_nulls_noop_copies_column(value):
ser1 = cudf.Series([value])
ser2 = ser1.nans_to_nulls()
assert ser1._column is not ser2._column


@pytest.mark.parametrize("dropna", [False, True])
def test_nunique_all_null(dropna):
data = [np.nan, np.nan]
pd_ser = pd.Series(data)
cudf_ser = cudf.Series(data, nan_as_null=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we are actually testing nan case here or do you want to test only NA cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hoping to just test the NA case here i.e. where dropna would impact the nunique count

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, okay. Can we use None then instead of np.nan?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing. Changed to use None

result = pd_ser.nunique(dropna=dropna)
expected = cudf_ser.nunique(dropna=dropna)
assert result == expected
Loading