Remove deprecation warning in TargetEncoder#7892
Merged
rapids-bot[bot] merged 2 commits intorapidsai:release/26.04from Mar 13, 2026
Merged
Remove deprecation warning in TargetEncoder#7892rapids-bot[bot] merged 2 commits intorapidsai:release/26.04from
TargetEncoder#7892rapids-bot[bot] merged 2 commits intorapidsai:release/26.04from
Conversation
This removes the deprecated 1D output handling in `TargetEncoder`.
jcrist
added
improvement
📝 WalkthroughSummary by CodeRabbit
WalkthroughRemoved a module-level deprecation warning from TargetEncoder and made output handling consistently return 2D numpy arrays; tests updated to request Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 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)
📝 Coding Plan
Comment Tip CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required. |
![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 the current code and only fix it if needed.
Inline comments:
In `@python/cuml/tests/test_target_encoder.py`:
- Line 91: Replace the unsafe eval usage when calling a cupy statistical
function: instead of eval(f"cp.{stat}")(y).item(), use getattr(cp, stat) to look
up the attribute at runtime and then call it with y (i.e. call getattr(cp,
stat)(y).item()). Update the expression in the assignment to answer to use
getattr(cp, stat) with the same arguments and .item() behavior so behavior
remains identical.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3cc892ff-dc1f-4d66-b46f-3323884baf08
📒 Files selected for processing (2)
python/cuml/cuml/preprocessing/TargetEncoder.pypython/cuml/tests/test_target_encoder.py
jcrist
commented
Mar 13, 2026
csadorf
approved these changes
Mar 13, 2026
There was a problem hiding this comment.
🧹 Nitpick comments (2)
python/cuml/tests/test_target_encoder.py (2)
284-289: Use NumPy for expected values whenoutput_type="numpy"is set.Line 288 currently builds
answith CuPy, which works but is a bit inconsistent with the explicit host-output expectation.Small consistency tweak
- ans = cp.asarray([0, 1, 0.5, 0.5])[:, None] + ans = np.array([0, 1, 0.5, 0.5])[:, None]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/cuml/tests/test_target_encoder.py` around lines 284 - 289, The expected array for the test should be created with NumPy rather than CuPy when TargetEncoder is instantiated with output_type="numpy"; change the construction of ans in the test_target_encoder.py snippet (where t_enc = TargetEncoder(output_type="numpy"), t_enc.fit(X, y), train_encoded = t_enc.transform(X)) to build ans using numpy (e.g., numpy.asarray) so the comparison via array_equal compares host arrays consistently with the transform output.
29-317: Consider centralizing the temporaryoutput_type="numpy"setup.This pattern is repeated many times; a tiny helper (or fixture) would make the eventual
#7893revert a one-liner and reduce churn in this file.Refactor sketch
+def _te_numpy(**kwargs): + return TargetEncoder(output_type="numpy", **kwargs) ... - encoder = TargetEncoder(output_type="numpy") + encoder = _te_numpy() ... - encoder = TargetEncoder(stat="median", output_type="numpy") + encoder = _te_numpy(stat="median")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/cuml/tests/test_target_encoder.py` around lines 29 - 317, Many tests repeatedly pass TargetEncoder(output_type="numpy"); create a small helper or pytest fixture (e.g., numpy_encoder or target_encoder_numpy) that returns a TargetEncoder configured with output_type="numpy" and replace direct constructions in tests like test_targetencoder_fit_transform, test_targetencoder_transform, test_targetencoder_multi_column, test_targetencoder_newly_encountered, test_one_category, test_targetencoder_pandas, test_targetencoder_numpy, test_targetencoder_cupy, test_targetencoder_smooth, test_targetencoder_customized_fold_id, test_targetencoder_var, test_transform_with_index, and test_targetencoder_median with calls to that helper/fixture to centralize the configuration and make future changes (like reverting `#7893`) a one-line update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@python/cuml/tests/test_target_encoder.py`:
- Around line 284-289: The expected array for the test should be created with
NumPy rather than CuPy when TargetEncoder is instantiated with
output_type="numpy"; change the construction of ans in the
test_target_encoder.py snippet (where t_enc =
TargetEncoder(output_type="numpy"), t_enc.fit(X, y), train_encoded =
t_enc.transform(X)) to build ans using numpy (e.g., numpy.asarray) so the
comparison via array_equal compares host arrays consistently with the transform
output.
- Around line 29-317: Many tests repeatedly pass
TargetEncoder(output_type="numpy"); create a small helper or pytest fixture
(e.g., numpy_encoder or target_encoder_numpy) that returns a TargetEncoder
configured with output_type="numpy" and replace direct constructions in tests
like test_targetencoder_fit_transform, test_targetencoder_transform,
test_targetencoder_multi_column, test_targetencoder_newly_encountered,
test_one_category, test_targetencoder_pandas, test_targetencoder_numpy,
test_targetencoder_cupy, test_targetencoder_smooth,
test_targetencoder_customized_fold_id, test_targetencoder_var,
test_transform_with_index, and test_targetencoder_median with calls to that
helper/fixture to centralize the configuration and make future changes (like
reverting `#7893`) a one-line update.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 361286df-8a63-4648-8a03-49095954bd0f
📒 Files selected for processing (1)
python/cuml/tests/test_target_encoder.py
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
3 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.
This removes the deprecated 1D output handling in
TargetEncoder.Fixes #7890.