Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
11 changes: 11 additions & 0 deletions python/cudf/cudf/core/groupby/groupby.py
Original file line number Diff line number Diff line change
Expand Up @@ -1273,6 +1273,17 @@ def agg(self, func=None, *args, engine=None, engine_kwargs=None, **kwargs):
)
elif agg_kind == "NUNIQUE":
cast_dtype = np.dtype(np.int64)
elif (
agg_name in {"cumsum", "cumprod"}
and is_pandas_nullable_extension_dtype(orig_dtype)
and orig_dtype.kind in {"i", "u"}
):
# libcudf's SUM/PRODUCT scans promote narrow integers
# to 64-bit. pandas does the same for numpy dtypes
# (int8 -> int64, GH#37493) but preserves masked
# extension dtypes (Int16 stays Int16, GH#58811),
# wrapping on overflow.
cast_dtype = orig_dtype
elif (
(
isinstance(agg_name, str)
Expand Down
8 changes: 0 additions & 8 deletions python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -1838,20 +1838,13 @@ def pytest_unconfigure(config):
"tests/groupby/test_groupby.py::test_groupby_cumsum_mask[Float32-False-val1]": "TODO: Add a reason for failure",
"tests/groupby/test_groupby.py::test_groupby_cumsum_mask[Float64-False-val1]": "TODO: Add a reason for failure",
"tests/groupby/test_groupby.py::test_groupby_cumsum_mask[Int16-False-val1]": "TODO: Add a reason for failure",
"tests/groupby/test_groupby.py::test_groupby_cumsum_mask[Int16-True-3]": "TODO: Add a reason for failure",
"tests/groupby/test_groupby.py::test_groupby_cumsum_mask[Int32-False-val1]": "TODO: Add a reason for failure",
"tests/groupby/test_groupby.py::test_groupby_cumsum_mask[Int32-True-3]": "TODO: Add a reason for failure",
"tests/groupby/test_groupby.py::test_groupby_cumsum_mask[Int64-False-val1]": "TODO: Add a reason for failure",
"tests/groupby/test_groupby.py::test_groupby_cumsum_mask[Int8-False-val1]": "TODO: Add a reason for failure",
"tests/groupby/test_groupby.py::test_groupby_cumsum_mask[Int8-True-3]": "TODO: Add a reason for failure",
"tests/groupby/test_groupby.py::test_groupby_cumsum_mask[UInt16-False-val1]": "TODO: Add a reason for failure",
"tests/groupby/test_groupby.py::test_groupby_cumsum_mask[UInt16-True-3]": "TODO: Add a reason for failure",
"tests/groupby/test_groupby.py::test_groupby_cumsum_mask[UInt32-False-val1]": "TODO: Add a reason for failure",
"tests/groupby/test_groupby.py::test_groupby_cumsum_mask[UInt32-True-3]": "TODO: Add a reason for failure",
"tests/groupby/test_groupby.py::test_groupby_cumsum_mask[UInt64-False-val1]": "TODO: Add a reason for failure",
"tests/groupby/test_groupby.py::test_groupby_cumsum_mask[UInt64-True-3]": "TODO: Add a reason for failure",
"tests/groupby/test_groupby.py::test_groupby_cumsum_mask[UInt8-False-val1]": "TODO: Add a reason for failure",
"tests/groupby/test_groupby.py::test_groupby_cumsum_mask[UInt8-True-3]": "TODO: Add a reason for failure",
"tests/groupby/test_groupby.py::test_groupby_cumsum_skipna_false": "TODO: Add a reason for failure",
"tests/groupby/test_groupby.py::test_groupby_cumsum_timedelta64": "TODO: Add a reason for failure",
"tests/groupby/test_groupby.py::test_groupby_groups_in_BaseGrouper": "TODO: Add a reason for failure",
Expand Down Expand Up @@ -1918,7 +1911,6 @@ def pytest_unconfigure(config):
"tests/groupby/test_reductions.py::test_sum_skipna_object[False]": "Inherent cudf.pandas None-vs-NaN difference for object-dtype null (skipna logic is correct)",
"tests/groupby/test_timegrouper.py::TestGroupBy::test_groupby_with_timegrouper": "TODO: Add a reason for failure",
"tests/groupby/test_timegrouper.py::TestGroupBy::test_scalar_call_versus_list_call": "TODO: Add a reason for failure",
"tests/groupby/transform/test_transform.py::test_nan_in_cumsum_group_label": "AssertionError: Attributes of Series are different",
"tests/indexes/base_class/test_reshape.py::TestReshape::test_insert_missing[Decimal]": "TODO: Add a reason for failure",
"tests/indexes/categorical/test_astype.py::TestAstype::test_categorical_date_roundtrip[False]": "TODO: Add a reason for failure",
"tests/indexes/categorical/test_astype.py::TestAstype::test_categorical_date_roundtrip[True]": "TODO: Add a reason for failure",
Expand Down
26 changes: 25 additions & 1 deletion python/cudf/cudf/tests/groupby/test_cummulative.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# SPDX-FileCopyrightText: Copyright (c) 2025, NVIDIA CORPORATION.
# SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# SPDX-License-Identifier: Apache-2.0
import numpy as np
import pandas as pd
Expand Down Expand Up @@ -105,3 +105,27 @@ def test_scan_int_null_pandas_compatible(op):
with cudf.option_context("mode.pandas_compatible", True):
result = getattr(df_cudf.groupby("b")["a"], op)()
assert_eq(result, expected)


@pytest.mark.parametrize("op", ["cumsum", "cumprod"])
def test_groupby_cumscan_masked_dtype_preserved(op):
# pandas preserves masked extension dtypes for groupby cum-scans
# (Int16 stays Int16, GH#58811) while numpy ints promote to 64-bit
pdf = pd.DataFrame({"a": [1, 1, 2], "b": [1, pd.NA, 2]}, dtype="Int16")
gdf = cudf.DataFrame(pdf)

expected = getattr(pdf.groupby("a")["b"], op)()
result = getattr(gdf.groupby("a")["b"], op)()

assert_eq(expected, result)


def test_groupby_cumsum_numpy_dtype_promotes():
# numpy int8 promotes to int64 (pandas GH#37493)
pdf = pd.DataFrame({"a": [1, 1], "b": [111, 111]}, dtype="int8")
gdf = cudf.DataFrame(pdf)

expected = pdf.groupby("a").cumsum()
result = gdf.groupby("a").cumsum()

assert_eq(expected, result)
Comment on lines +110 to +131

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift

Add a unit benchmark for the new casting path.

The changed files add correctness tests but no benchmark for this feature/bug fix. Add a benchmark covering nullable-integer cumsum/cumprod, ideally including representative group sizes and overflow cases, to track the cost of the post-scan cast.

As per coding guidelines, “Add unit tests and unit benchmarks for feature and bug-fix contributions.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@python/cudf/cudf/tests/groupby/test_cummulative.py` around lines 110 - 131,
Add a unit benchmark alongside test_groupby_cumscan_masked_dtype_preserved
covering grouped nullable-integer cumsum and cumprod, with representative group
sizes and overflow-inducing values, so the post-scan casting path is measured
for both operations.

Source: Coding guidelines

Loading