Skip to content

FIX Remove spurious IsolationForest arguments - #8486

Merged
rapids-bot[bot] merged 2 commits into
NVIDIA:mainfrom
betatim:remove-spurious-args
Aug 18, 2026
Merged

FIX Remove spurious IsolationForest arguments#8486
rapids-bot[bot] merged 2 commits into
NVIDIA:mainfrom
betatim:remove-spurious-args

Conversation

@betatim

@betatim betatim commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

These arguments should not have been added, now they are gone. This removes the warm_start constructor argument that was unsupported and only ever raised. The same goes for the sample_weights argument used in fit() and friends.

No deprecation cycles because this fixes a bug (arguments should never have been there in the first place).

Towards #8420

These arguments should not have been added, now they are gone.
@betatim
betatim requested a review from a team as a code owner August 18, 2026 14:18
@betatim
betatim requested a review from jcrist August 18, 2026 14:18
@github-actions github-actions Bot added the Cython / Python Cython or Python issue label Aug 18, 2026
@betatim betatim added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Aug 18, 2026
@betatim betatim mentioned this pull request Aug 18, 2026
15 tasks
@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 56f95c2d-b060-4881-950f-ce7089267a98

📥 Commits

Reviewing files that changed from the base of the PR and between f738af6 and 7b9ba7c.

📒 Files selected for processing (1)
  • python/cuml/tests/test_isolation_forest.py
💤 Files with no reviewable changes (1)
  • python/cuml/tests/test_isolation_forest.py

Included review availability: Your plan includes up to 12 reviews per rolling hour; 11 remain after this review.


📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Updated Isolation Forest APIs to remove unsupported warm_start and sample_weight options.
    • Simplified fit and fit_predict usage by removing unsupported sample-weight arguments.
    • Updated documentation and compatibility checks to reflect the supported functionality.

Walkthrough

IsolationForest no longer exposes warm_start or sample_weight. Its constructor, fitting methods, documentation, and parameter mappings were updated. Obsolete rejection tests and sample-weight compatibility XFAILs were removed.

Changes

IsolationForest interface cleanup

Layer / File(s) Summary
Remove unsupported estimator interfaces
python/cuml/cuml/ensemble/isolation_forest.pyx
Removes warm_start from estimator configuration and removes sample_weight from fit and fit_predict.
Update unsupported-option tests
python/cuml/tests/test_isolation_forest.py, python/cuml/tests/test_sklearn_compatibility.py
Removes tests for unsupported direct-fitting arguments and sample-weight compatibility XFAILs.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 7b9ba

This PR removes unsupported IsolationForest arguments and updates the related tests; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the removal of unsupported IsolationForest arguments and links the change to issue #8420.
Title check ✅ Passed The title clearly and concisely describes the removal of unsupported IsolationForest arguments.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@jcrist jcrist added breaking Breaking change and removed non-breaking Non-breaking change labels Aug 18, 2026

@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.

Thanks! I've moved this to a breaking change just to note the arg removal (though I doubt anyone is relying on unsupported args from a just released estimator).

One small comment, do with it what you will.

Comment thread python/cuml/tests/test_isolation_forest.py Outdated
This should be tested in cuml.accel
@betatim

betatim commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the breaking label, it is the smarter choice.

@betatim

betatim commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot
rapids-bot Bot merged commit 8f1db4c into NVIDIA:main Aug 18, 2026
98 checks passed
@betatim
betatim deleted the remove-spurious-args branch August 19, 2026 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Breaking change Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants