Cleanup LabelBinarizer - #8101
Merged
rapids-bot[bot] merged 7 commits intoMay 19, 2026
Merged
Conversation
jcrist
added
improvement
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Member
Author
|
/ok to test e395005 |
44 tasks
jcrist
force-pushed
the
cleanup-label-binarizer
branch
from
May 14, 2026 17:59
e395005 to
ed0a53b
Compare
jcrist
marked this pull request as ready for review
May 14, 2026 18:00
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors label binarization for dense/sparse/multilabel inputs with GPU support, updates sparse dtype/memory coercion in check_array, exposes a cuml.accel LabelBinarizer proxy, updates docs, and adds unit/integration/import-export/validation tests. ChangesLabelBinarizer GPU acceleration and sklearn integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/cuml/cuml/accel/_overrides/sklearn/preprocessing.py`:
- Around line 62-67: The _gpu_inverse_transform in class LabelBinarizer is
ignoring the passed threshold by calling self._gpu.inverse_transform(Y,
threshold=None); change the call to forward the threshold parameter (e.g.,
self._gpu.inverse_transform(Y, threshold=threshold)) so the user-supplied
threshold is respected; update LabelBinarizer._gpu_inverse_transform accordingly
and preserve the existing signature to keep default behavior when threshold is
None.
🪄 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: 5049aa88-9de5-472d-aa7f-d7e1b8407e27
📒 Files selected for processing (8)
docs/source/cuml-accel/faq.rstdocs/source/cuml-accel/limitations.rstpython/cuml/cuml/accel/_overrides/sklearn/preprocessing.pypython/cuml/cuml/internals/validation.pypython/cuml/cuml/preprocessing/label.pypython/cuml/cuml_accel_tests/integration/test_preprocessing.pypython/cuml/tests/test_label_binarizer.pypython/cuml/tests/test_validation.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
python/cuml/tests/test_sklearn_import_export.py (1)
1051-1060: ⚡ Quick winAssert imported fitted attributes on
cu_model2directly.
cu_model2(from sklearn on Line 1051) isn’t directly validated fory_type_/sparse_input_. Current assertions only cover the cuML→sklearn→cuML path throughroundtrip.Suggested patch
cu_model2 = cuml.preprocessing.LabelBinarizer.from_sklearn(sk_model) sk_model2 = cu_model.as_sklearn() roundtrip = cuml.preprocessing.LabelBinarizer.from_sklearn(sk_model2) assert_roundtrip_consistency(cu_model, roundtrip) @@ np.testing.assert_array_equal(cu_model.classes_, sk_model2.classes_) np.testing.assert_array_equal(cu_model.classes_, roundtrip.classes_) + assert cu_model2.y_type_ == sk_model.y_type_ + assert cu_model2.sparse_input_ == sk_model.sparse_input_ assert roundtrip.y_type_ == cu_model.y_type_ assert roundtrip.sparse_input_ == cu_model.sparse_input_🤖 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/cuml/tests/test_sklearn_import_export.py` around lines 1051 - 1060, Add direct assertions to validate that the cuML model produced by importing the fitted sklearn model (cu_model2 from cuml.preprocessing.LabelBinarizer.from_sklearn(sk_model)) preserved fitted attributes; specifically, assert that cu_model2.classes_ equals cu_model.classes_ (or sk_model2.classes_), and assert cu_model2.y_type_ == cu_model.y_type_ and cu_model2.sparse_input_ == cu_model.sparse_input_. Keep the existing roundtrip checks (roundtrip and sk_model2) but add these direct assertions on cu_model2 to cover the sklearn→cuML import path.
🤖 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.
Nitpick comments:
In `@python/cuml/tests/test_sklearn_import_export.py`:
- Around line 1051-1060: Add direct assertions to validate that the cuML model
produced by importing the fitted sklearn model (cu_model2 from
cuml.preprocessing.LabelBinarizer.from_sklearn(sk_model)) preserved fitted
attributes; specifically, assert that cu_model2.classes_ equals
cu_model.classes_ (or sk_model2.classes_), and assert cu_model2.y_type_ ==
cu_model.y_type_ and cu_model2.sparse_input_ == cu_model.sparse_input_. Keep the
existing roundtrip checks (roundtrip and sk_model2) but add these direct
assertions on cu_model2 to cover the sklearn→cuML import path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 55416ea7-1fc2-40e8-9314-9395e4d92b7e
📒 Files selected for processing (1)
python/cuml/tests/test_sklearn_import_export.py
csadorf
requested changes
May 18, 2026
csadorf
left a comment
csadorf
left a comment
Contributor
There was a problem hiding this comment.
One minor behavioral divergence we should fix, otherwise LGTM. Fantastic work!
| LabelBinarizer | ||
| ^^^^^^^^^^^^^^ | ||
|
|
||
| ``LabelBinarizer`` supports all cases and will never fall back to CPU. |
Comment on lines
+118
to
+119
| # XXX: cudf's to_numpy doesn't support conversions for all | ||
| # dtypes. Roundtrip through object dtype when necessary. |
Contributor
There was a problem hiding this comment.
(suggestion not directly related to this pr) I think it would be worthwhile to catalog those and similar limitations and ensure that they are reported upstream.
jcrist
force-pushed
the
cleanup-label-binarizer
branch
from
May 18, 2026 17:09
a085d15 to
b4e3eca
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/cuml/tests/test_validation.py`:
- Around line 1005-1012: The pytest.raises match strings contain unescaped regex
metacharacters (the dots in "cupyx.scipy.sparse"), making the regex too
permissive; update the three pytest.raises calls that wrap check_array (those
passing host_i4, device_f4, and device_f4 with mem_type=None) to escape the dots
(e.g., "cupyx\\.scipy\\.sparse") or construct the pattern with re.escape so the
literal module path is matched precisely.
🪄 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: 46980743-efaa-428f-b1d5-4ca50dc319bd
📒 Files selected for processing (10)
docs/source/cuml-accel/faq.rstdocs/source/cuml-accel/limitations.rstpython/cuml/cuml/accel/_overrides/sklearn/preprocessing.pypython/cuml/cuml/internals/validation.pypython/cuml/cuml/preprocessing/label.pypython/cuml/cuml_accel_tests/integration/test_preprocessing.pypython/cuml/tests/test_label_binarizer.pypython/cuml/tests/test_naive_bayes.pypython/cuml/tests/test_sklearn_import_export.pypython/cuml/tests/test_validation.py
✅ Files skipped from review due to trivial changes (1)
- docs/source/cuml-accel/faq.rst
🚧 Files skipped from review as they are similar to previous changes (7)
- docs/source/cuml-accel/limitations.rst
- python/cuml/cuml/accel/_overrides/sklearn/preprocessing.py
- python/cuml/tests/test_naive_bayes.py
- python/cuml/cuml_accel_tests/integration/test_preprocessing.py
- python/cuml/tests/test_sklearn_import_export.py
- python/cuml/tests/test_label_binarizer.py
- python/cuml/cuml/preprocessing/label.py
csadorf
approved these changes
May 18, 2026
- Updates `LabelBinarizer` to follow standard cuml and sklearn conventions (simple __init__, no mutation, type reflection, ...) - Applies new validation - Adds `sparse_input_` and `y_type_` attributes - Improves validation and error messages - Improves resilience and sklearn compatibility - Adds support for sklearn interop - Adds support for cuml.accel - Improves docstrings
jcrist
force-pushed
the
cleanup-label-binarizer
branch
from
May 18, 2026 21:27
fb92192 to
3333b51
Compare
Member
Author
|
/merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
LabelBinarizerto follow standard cuml and sklearn conventions (simple__init__, no mutation, type reflection, ...)sparse_input_andy_type_attributesThis required one change to
cuml.internals.validationaround handling of unsupported dtypes forcupyx.scipy.sparse. This is split out into a separate commit with a new test case.Part of #7317.
Fixes #8087.