Fix a few lingering sklearn compat xfails

[jcrist]

• Mark check_dont_overwrite_parameters as failing due to TSNE not supporting n_components=1. The actual issue that test is checking for is already resolved in TSNE (no mutation in of hyperparameters).
• Move to explicit parameters in __init__ for all estimators. This fixes check_do_not_raise_errors_in_init_or_set_params in all remaining estimators.
• Move to eagerly loading _nvforest_model in RandomForestClassifier/RandomForestRegressor. I think this makes sense. After a fit, we should ensure the estimator is fully ready to do inference without prior additional work (which may fail). Fixes check_dict_unchanged for all remaining estimators.

Fixes #8165.

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• New Features

  • Constructors for RandomForest and KNeighbors estimators now expose explicit, scikit-learn‑style parameters with defaults for clearer API discoverability.
  • KNeighbors docstrings expanded to document additional init parameters and compatibility notes.

• Bug Fixes

  • Improved parameter handling and initialization behavior for RandomForest and KNeighbors to enhance sklearn compatibility.

• Chores

  • Updated test compatibility expectations for the affected estimators.

Walkthrough

Refactors RandomForest and KNeighbors constructors to explicit sklearn-compatible keyword-only parameters and defaults. Removes deprecated RandomForest criterion parameter. _fit_forest now caches the nvforest model immediately after Treelite serialization. Test XFAIL entries are reduced to match the new constructor behavior.

Changes

Estimator API Clarification and Sklearn Compatibility

Layer / File(s)	Summary
RandomForest base class: remove criterion parameter and update model caching python/cuml/cuml/ensemble/randomforest_common.pyx	BaseRandomForestModel.__init__ removes the criterion parameter; _fit_forest immediately caches the nvforest model via self.as_nvforest() after Treelite serialization instead of resetting _nvforest_model to None.
RandomForestClassifier explicit hyperparameters python/cuml/cuml/ensemble/randomforestclassifier.py	RandomForestClassifier.__init__ declares core hyperparameters (n_estimators, split_criterion="gini", bootstrap, max_depth="deprecated", etc.) as explicit keyword-only arguments and passes them to BaseRandomForestModel.
RandomForestRegressor explicit hyperparameters python/cuml/cuml/ensemble/randomforestregressor.py	RandomForestRegressor.__init__ declares core hyperparameters explicitly with regression-appropriate defaults (split_criterion="mse", max_features=1.0, max_depth="deprecated") and passes them to BaseRandomForestModel.
NeighborsBase parameter order normalization python/cuml/cuml/neighbors/nearest_neighbors.pyx	NeighborsBase.__init__ reorders keyword-only parameters so verbose appears after n_jobs; no behavioral changes.
KNeighborsClassifier explicit sklearn parameters python/cuml/cuml/neighbors/kneighbors_classifier.pyx	Docstring extended and __init__ now exposes sklearn-compatible parameters (n_neighbors=5, algorithm="auto", metric="euclidean", p=2, algo_params, metric_params, n_jobs) and forwards them to super().__init__.
KNeighborsRegressor explicit sklearn parameters python/cuml/cuml/neighbors/kneighbors_regressor.pyx	Docstring extended and __init__ now exposes sklearn-compatible parameters (n_neighbors=5, algorithm="auto", metric="euclidean", p=2, algo_params, metric_params, n_jobs) and forwards them to super().__init__.
Update sklearn compatibility XFAILS python/cuml/tests/test_sklearn_compatibility.py	Reduced per-estimator XFAIL entries for RandomForestClassifier, RandomForestRegressor, KNeighborsClassifier, and KNeighborsRegressor (removed several init/set_params/dict-mutation xfails); updated TSNE xfail message string.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• rapidsai/cuml#8126: Adjusts test_sklearn_compatibility.py XFAIL configuration for RandomForest* and KNeighbors* estimators.
• rapidsai/cuml#7753: Modifies the same test configuration file and XFAIL entries for sklearn compatibility.
• rapidsai/cuml#8019: Updates test_sklearn_compatibility.py XFAIL adjustments related to KNeighbors* and neighbor estimators.

Suggested labels

Cython / Python

Suggested reviewers

• csadorf
• divyegala

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% 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 'Fix a few lingering sklearn compat xfails' clearly and concisely summarizes the main objective of removing remaining sklearn compatibility xfails by fixing lingering issues.
Description check	✅ Passed	The description comprehensively explains the three main changes: addressing TSNE check_dont_overwrite_parameters issue, converting to explicit parameters for init, and eagerly loading nvforest model.
Linked Issues check	✅ Passed	The PR fully addresses issue #8165 objectives by fixing all four sklearn checks (check_do_not_raise_errors_in_init_or_set_params, check_dont_overwrite_parameters, check_dict_unchanged) through explicit parameters and eager model loading.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing sklearn compatibility xfails: parameter signature refactoring, nvforest model caching, and xfail configuration updates align with issue #8165 objectives.

_{✏️ 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.}

[coderabbitai]

Actionable comments posted: 2

🤖 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/neighbors/kneighbors_classifier.pyx`:
- Around line 154-178: Update the class/docstring for the KNeighborsClassifier
constructor to document the newly exposed parameters: p, algo_params,
metric_params, and n_jobs (even if n_jobs is ignored for compatibility). Modify
the docstring associated with __init__ (or the class-level documentation in
kneighbors_classifier.pyx) to add short descriptions for each symbol (p:
Minkowski power parameter, algo_params: algorithm-specific options,
metric_params: additional metric args, n_jobs: parallelism placeholder) and
ensure the parameter list matches the signature shown in __init__.

In `@python/cuml/cuml/neighbors/kneighbors_regressor.pyx`:
- Around line 156-180: The class docstring for KNeighborsRegressor (file:
kneighbors_regressor.pyx) is out of sync with the expanded __init__ signature;
update the class-level docstring to document the newly exposed parameters p,
algo_params, metric_params, and n_jobs: add brief descriptions (type, default,
purpose) for p (Minkowski power parameter), algo_params and metric_params (dicts
of algorithm/metric-specific options), and n_jobs (int or None, parallelism
placeholder for sklearn compatibility), ensuring the parameter list in the
docstring matches the parameters of __init__.

🪄 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: a8126dd0-6d9c-4666-a24f-6323a9527d9e

📥 Commits

Reviewing files that changed from the base of the PR and between 796fa15 and 664cbc8.

📒 Files selected for processing (7)

• python/cuml/cuml/ensemble/randomforest_common.pyx
• python/cuml/cuml/ensemble/randomforestclassifier.py
• python/cuml/cuml/ensemble/randomforestregressor.py
• python/cuml/cuml/neighbors/kneighbors_classifier.pyx
• python/cuml/cuml/neighbors/kneighbors_regressor.pyx
• python/cuml/cuml/neighbors/nearest_neighbors.pyx
• python/cuml/tests/test_sklearn_compatibility.py

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

python/cuml/cuml/neighbors/kneighbors_regressor.pyx (1)

147-148: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix incorrect sklearn reference in Notes section.

The docstring references KNeighborsClassifier but this is the KNeighborsRegressor class. The link text and URL should reference the regressor instead.

📝 Proposed fix

-    For additional docs, see `scikitlearn's KNeighborsClassifier
-    <https://scikit-learn.org/stable/modules/generated/sklearn.neighbors.KNeighborsClassifier.html>`_.
+    For additional docs, see `scikitlearn's KNeighborsRegressor
+    <https://scikit-learn.org/stable/modules/generated/sklearn.neighbors.KNeighborsRegressor.html>`_.

🤖 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/cuml/neighbors/kneighbors_regressor.pyx` around lines 147 - 148,
Update the docstring in the KNeighborsRegressor implementation to reference the
regressor docs instead of the classifier: change the link text and URL that
currently point to `sklearn.neighbors.KNeighborsClassifier` to
`sklearn.neighbors.KNeighborsRegressor` (e.g., update the link target and
visible text in the docstring inside
python/cuml/cuml/neighbors/kneighbors_regressor.pyx where the class
KNeighborsRegressor is documented).

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

Outside diff comments:
In `@python/cuml/cuml/neighbors/kneighbors_regressor.pyx`:
- Around line 147-148: Update the docstring in the KNeighborsRegressor
implementation to reference the regressor docs instead of the classifier: change
the link text and URL that currently point to
`sklearn.neighbors.KNeighborsClassifier` to
`sklearn.neighbors.KNeighborsRegressor` (e.g., update the link target and
visible text in the docstring inside
python/cuml/cuml/neighbors/kneighbors_regressor.pyx where the class
KNeighborsRegressor is documented).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d1e8cb42-8461-480c-8243-aac82fbb1377

📥 Commits

Reviewing files that changed from the base of the PR and between 664cbc8 and 40f7e40.

📒 Files selected for processing (2)

• python/cuml/cuml/neighbors/kneighbors_classifier.pyx
• python/cuml/cuml/neighbors/kneighbors_regressor.pyx

[betatim]

Thanks for this cleanup and improvement!

Have you asked who ever did the original implementation what they think about the extra time spent in fit doing the loading? I agree it makes more sense to do it then and there, but it must cost a bit of time and I assume the original author had some reason to add the complexity of making it lazy?

[jcrist]

I was the original author 😉. I did that more for simplicity than anything, but now think this change makes more sense.

[jcrist]

/merge

[betatim]

I was the original author 😉. I did that more for simplicity than anything, but now think this change makes more sense.

heh - a rare case of future you not regretting past you's choices.