Remove deprecated .values_host property in cudf - #22649
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR removes the deprecated ChangesDeprecation Removal: values_host → to_numpy()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
python/cudf/cudf/tests/indexes/multiindex/methods/test_to_numpy.py (1)
8-17:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd edge-path cases for
MultiIndex.to_numpy().Please add empty and null-containing MultiIndex cases here; the current test only validates one nominal input.
As per coding guidelines
python/**/test_*.py: Ensure test files provide comprehensive edge case coverage (empty, all-null, single-element, mixed types) and do not depend on external datasets.🤖 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/indexes/multiindex/methods/test_to_numpy.py` around lines 8 - 17, The test_multiindex_to_numpy currently only checks a single nominal MultiIndex; add additional edge-path cases to cover empty, all-null, and single-element/mixed-type MultiIndex inputs for MultiIndex.to_numpy(). Create new MultiIndex instances using cudf.MultiIndex (e.g., an empty MultiIndex with empty levels/codes, a MultiIndex where codes contain -1/null positions to produce null entries, and small single-element or mixed-type levels), convert each to pandas via .to_pandas() and compare midx.to_numpy() with pmidx.values using assert_eq to validate behavior across those edge cases.python/cudf/cudf/tests/indexes/index/methods/test_to_numpy.py (1)
11-17:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winExpand edge-case coverage for
Index.to_numpy()tests.Current parametrization only covers empty and single-element inputs; please add explicit all-null and mixed-type cases in this module to match the test coverage standard.
As per coding guidelines
python/**/test_*.py: Ensure test files provide comprehensive edge case coverage (empty, all-null, single-element, mixed types) and do not depend on external datasets.🤖 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/indexes/index/methods/test_to_numpy.py` around lines 11 - 17, Update the test_index_to_numpy parametrization to include explicit all-null and mixed-type inputs so edge cases are covered: add cases like a multi-null list (e.g., [None, None]) to validate all-null behavior and a mixed list (e.g., [1, None] or ["a", 1]) to validate mixed-type handling; in the test_index_to_numpy function use the existing cudf.Index (gdi) and pd.Index (pdi) construction with the same dtype parameter (all_supported_types_as_str) and assert equality via np.testing.assert_array_equal(gdi.to_numpy(), pdi.values) for those new inputs as well, ensuring mixed-type cases use an appropriate dtype (e.g., object) if needed so pandas and cudf Index creation succeed.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@python/cudf/cudf/tests/series/methods/test_to_numpy.py`:
- Around line 39-53: Add explicit all-null and single-element cases to the
test_series_to_numpy parametrization so the test covers empty, all-null,
single-element and mixed inputs. Modify the "data" list in test_series_to_numpy
to include an all-null case (e.g., [None, None] or [np.nan, np.nan]) and a
single-element case (e.g., [1] or ["x"]) alongside the existing examples,
keeping the existing dtype logic (None if data else float) and the asserts that
compare pds.values to gds.to_numpy().
---
Outside diff comments:
In `@python/cudf/cudf/tests/indexes/index/methods/test_to_numpy.py`:
- Around line 11-17: Update the test_index_to_numpy parametrization to include
explicit all-null and mixed-type inputs so edge cases are covered: add cases
like a multi-null list (e.g., [None, None]) to validate all-null behavior and a
mixed list (e.g., [1, None] or ["a", 1]) to validate mixed-type handling; in the
test_index_to_numpy function use the existing cudf.Index (gdi) and pd.Index
(pdi) construction with the same dtype parameter (all_supported_types_as_str)
and assert equality via np.testing.assert_array_equal(gdi.to_numpy(),
pdi.values) for those new inputs as well, ensuring mixed-type cases use an
appropriate dtype (e.g., object) if needed so pandas and cudf Index creation
succeed.
In `@python/cudf/cudf/tests/indexes/multiindex/methods/test_to_numpy.py`:
- Around line 8-17: The test_multiindex_to_numpy currently only checks a single
nominal MultiIndex; add additional edge-path cases to cover empty, all-null, and
single-element/mixed-type MultiIndex inputs for MultiIndex.to_numpy(). Create
new MultiIndex instances using cudf.MultiIndex (e.g., an empty MultiIndex with
empty levels/codes, a MultiIndex where codes contain -1/null positions to
produce null entries, and small single-element or mixed-type levels), convert
each to pandas via .to_pandas() and compare midx.to_numpy() with pmidx.values
using assert_eq to validate behavior across those edge cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b3cc4c01-2f26-46f2-841f-f496384846fc
📒 Files selected for processing (17)
docs/cudf/source/cudf/api_docs/dataframe.rstdocs/cudf/source/cudf/api_docs/index_objects.rstdocs/cudf/source/cudf/api_docs/series.rstdocs/cudf/source/cudf/developer_guide/library_design.mdpython/cudf/benchmarks/API/bench_frame_or_index.pypython/cudf/benchmarks/API/bench_rangeindex.pypython/cudf/cudf/core/column/column.pypython/cudf/cudf/core/frame.pypython/cudf/cudf/core/index.pypython/cudf/cudf/core/multiindex.pypython/cudf/cudf/core/single_column_frame.pypython/cudf/cudf/tests/indexes/index/methods/test_to_numpy.pypython/cudf/cudf/tests/indexes/index/test_attributes.pypython/cudf/cudf/tests/indexes/multiindex/methods/test_to_numpy.pypython/cudf/cudf/tests/indexes/multiindex/test_attributes.pypython/cudf/cudf/tests/series/methods/test_to_numpy.pypython/cudf/cudf/tests/series/test_attributes.py
💤 Files with no reviewable changes (11)
- docs/cudf/source/cudf/api_docs/series.rst
- docs/cudf/source/cudf/api_docs/dataframe.rst
- docs/cudf/source/cudf/api_docs/index_objects.rst
- python/cudf/benchmarks/API/bench_rangeindex.py
- python/cudf/cudf/core/column/column.py
- python/cudf/benchmarks/API/bench_frame_or_index.py
- python/cudf/cudf/tests/indexes/multiindex/test_attributes.py
- python/cudf/cudf/core/frame.py
- python/cudf/cudf/tests/series/test_attributes.py
- python/cudf/cudf/tests/indexes/index/test_attributes.py
- python/cudf/cudf/core/single_column_frame.py
|
/merge |
Description
Deprecated in 26.04
Checklist