Skip to content

Migrate _classification and _ranking metrics to new input validation infrastructure - #8012

Merged
rapids-bot[bot] merged 2 commits into
NVIDIA:mainfrom
csadorf:metrics-new-validation
Apr 28, 2026
Merged

Migrate _classification and _ranking metrics to new input validation infrastructure#8012
rapids-bot[bot] merged 2 commits into
NVIDIA:mainfrom
csadorf:metrics-new-validation

Conversation

@csadorf

@csadorf csadorf commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Migrate _classification and _ranking metrics to new input validation infrastructure

Part of #7998
Part of #7428

@csadorf
csadorf requested a review from a team as a code owner April 23, 2026 22:37
@csadorf
csadorf requested a review from divyegala April 23, 2026 22:37
@github-actions github-actions Bot added the Cython / Python Cython or Python issue label Apr 23, 2026
@csadorf csadorf added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change and removed Cython / Python Cython or Python issue labels Apr 23, 2026
@coderabbitai

coderabbitai Bot commented Apr 23, 2026

Copy link
Copy Markdown
📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Improved input validation for classification and ranking metric functions to enforce consistent sample lengths and valid shapes.
    • Better error messages for mismatched sample dimensions.
    • Support for scalar sample_weight values; 2D sample_weight inputs are rejected.
  • Tests

    • Updated tests to reflect new validation behavior and removed several expected-failure entries for neighbor LOOCV classifier tests.

Walkthrough

Refactors cuML metrics input validation to use scikit-learn utilities (check_array, check_consistent_length, check_sample_weight) across classification and ranking metrics; updates tests to match new error messages and validation behavior; removes four entries from the cuML accel xfail YAML list.

Changes

Cohort / File(s) Summary
Classification Metrics
python/cuml/cuml/metrics/_classification.py
Replaced input_to_cupy_array usage with check_array, check_consistent_length, and check_sample_weight. _input_to_cupy_or_cudf_series now coerces to 1D via check_array (falls back to cudf.Series), squeezes single-column arrays, and removes explicit row-count enforcement and negative-value checks.
Ranking Metrics
python/cuml/cuml/metrics/_ranking.py
Switched precision_recall_curve and roc_auc_score input handling to check_array(ensure_2d=False) with an explicit dtype whitelist and added check_consistent_length(y_true, y_score). Updated SPDX year.
Tests
python/cuml/tests/test_metrics.py
Adjusted test_accuracy_score_errors to expect new error messages, removed a sample-weight matrix case from a loop, added assertion that 2D sample_weight is rejected, and added a test that scalar sample_weight is accepted and unchanged behavior for normalize variants.
xfail YAML
python/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-list.yaml
Removed four xfail entries related to LOOCV neighbor classifier tests (parameterized auto, ball_tree, brute, kd_tree) so those cases are no longer marked as expected failures.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: migrating classification and ranking metrics from old input validation to new validation infrastructure.
Description check ✅ Passed The description directly relates to the changeset, clearly stating the migration of metrics to new input validation infrastructure and referencing relevant GitHub issues.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@csadorf
csadorf force-pushed the metrics-new-validation branch from 293b8b7 to e002c66 Compare April 24, 2026 15:49
@github-actions github-actions Bot added the Cython / Python Cython or Python issue label Apr 24, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@python/cuml/cuml/metrics/_classification.py`:
- Around line 37-42: The broad except (ValueError, TypeError) is converting
validation errors into a cudf.Series fallback; change the logic so we only fall
back to cudf for unsupported label dtypes (e.g., object/str/categorical) and
re-raise for dimensionality/shape/type validation errors: before creating
cudf.Series(x, ...), inspect x's dtype (e.g., via numpy.asarray(x).dtype or
pandas.api.types.is_object_dtype / is_categorical_dtype) and if it indicates an
unsupported label dtype then execute the cudf fallback (assign to out),
otherwise let the original exception propagate (re-raise) so functions like
whatever calls this code continue to enforce shape/dimensionality validation.
Ensure you reference x and out in the change.
🪄 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: ef3dae8c-8622-432a-b744-2860f7de983a

📥 Commits

Reviewing files that changed from the base of the PR and between 293b8b7 and e002c66.

📒 Files selected for processing (4)
  • python/cuml/cuml/metrics/_classification.py
  • python/cuml/cuml/metrics/_ranking.py
  • python/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-list.yaml
  • python/cuml/tests/test_metrics.py
💤 Files with no reviewable changes (1)
  • python/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-list.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
  • python/cuml/tests/test_metrics.py
  • python/cuml/cuml/metrics/_ranking.py

Comment thread python/cuml/cuml/metrics/_classification.py

@jcrist jcrist left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:shipit:

@jcrist

jcrist commented Apr 28, 2026

Copy link
Copy Markdown
Member

/merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants