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
7 changes: 7 additions & 0 deletions python/cudf/cudf/core/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,12 @@ def _to_array(
# Internal function to implement to_cupy and to_numpy, which are nearly
# identical except for the attribute they access to generate values.

def is_numpy_object_dtype(dtype: Dtype | None) -> bool:
try:
return np.dtype(dtype) == np.dtype("O")
except TypeError:
return False
Comment on lines +630 to +634

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift

Add a regression benchmark for the updated object-dtype null path.

This path changes core conversion behavior in Frame._to_array; please add a unit benchmark that covers to_numpy(dtype=object) with nulls (default na_value and explicit na_value) to guard against performance regressions on this hot conversion route.

As per coding guidelines **/*.{cpp,cu,py,pyx}: Add unit tests and unit benchmarks for code contributions.

Also applies to: 713-716

🤖 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/core/frame.py` around lines 656 - 660, Add a regression
benchmark that measures the hot conversion path exercised by Frame._to_array /
DataFrame.to_numpy(dtype=object) when nulls are present: create a
pytest-benchmark (or project-standard benchmarking harness) test that constructs
frames with nulls and calls to_numpy(dtype=object) twice—once using the default
na_value and once passing an explicit na_value—to capture performance for both
paths; name the test clearly (e.g., bench_to_numpy_object_with_nulls) and place
it with other Python benchmarks so future changes to is_numpy_object_dtype /
Frame._to_array will be covered.


def to_array(
col: ColumnBase, to_dtype: Dtype | None
) -> cupy.ndarray | np.ndarray:
Expand Down Expand Up @@ -678,6 +684,7 @@ def to_array(
col.has_nulls()
and dtype is not None
and is_string_dtype(dtype)
and not is_numpy_object_dtype(dtype)
):
casted_array[col.isnull().to_numpy()] = (
cudf.NA if na_value is no_default else na_value
Expand Down
32 changes: 32 additions & 0 deletions python/cudf/cudf/tests/dataframe/methods/test_to_cupy.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,3 +195,35 @@ def test_to_numpy_object_dtype_boxes_values(constructor, data, dtype):
# Ensure boxed values are real Python scalars, not strings.
for value in result.flat:
assert not isinstance(value, str)


@pytest.mark.parametrize("constructor", ["DataFrame", "Series"])
@pytest.mark.parametrize(
"data",
[
[],
[None],
[None, None],
["a", "b", None],
],
)
def test_to_numpy_object_dtype_preserves_none_string_nulls(constructor, data):
values = pd.Series(data, dtype=object, name="x")
if constructor == "DataFrame":
pd_obj = pd.DataFrame({"x": values})
else:
pd_obj = values
cudf_obj = getattr(cudf, constructor)(pd_obj)

expected = pd_obj.to_numpy(dtype=object)
result = cudf_obj.to_numpy(dtype=object)
null_mask = pd.isna(expected)

assert result.dtype == np.dtype("O")
np.testing.assert_array_equal(result, expected)
assert all(value is None for value in result[null_mask].flat)

expected = pd_obj.to_numpy(dtype=object, na_value="missing")
result = cudf_obj.to_numpy(dtype=object, na_value="missing")
np.testing.assert_array_equal(result, expected)
assert all(value == "missing" for value in result[null_mask].flat)
Loading