Support subclassing proxy estimators

[jcrist]

This adds support for subclassing proxy estimators in cuml.accel. Subclasses of these estimators are not accelerated, since a subclass of a sklearn estimator may be built on the internals (and not the proxied public interface).

To accomplish this, we add a new ProxyBaseMeta metaclass for ProxyBase. This metaclass detects when a subclass of a concrete proxy estimator is made, and swaps out the proxy classes for the concrete CPU classes instead in the subclass's MRO. It also overrides __subclasscheck__ and __instancecheck__ to ensure that even though the proxy class isn't in the MRO, subclasses and instances still report properly for isinstance and issubclass.

This has the downside that metaclasses don't compose as well as __init_subclass__. The metaclass of a new class must be a (non-strict) subclass of the metaclass of all base classes. This addition means that if a user subclasses a proxy estimator and also adds in another custom metaclass, they'll still get an error. To avoid this for common cases, ProxyBaseMeta derives from abc.ABCMeta, which is maybe the most common user-used metaclass.

Doing this lets imblearn be imported and used with cuml.accel active. It's a best-effort fix, and I think should be sufficient for most use cases.

Fixes #7971.

[coderabbitai]

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: 8fef4a00-c4a6-4223-a570-dee014b08b10

📥 Commits

Reviewing files that changed from the base of the PR and between a068cb7 and 9e75b28.

📒 Files selected for processing (3)

• python/cuml/cuml/accel/estimator_proxy.py
• python/cuml/cuml_accel_tests/test_estimator_proxy.py
• python/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-list.yaml

---

📝 Walkthrough

Summary by CodeRabbit

• Refactor

  • Improved proxy detection logic for more consistent proxy/subclass behavior and prevented generation of proxy wrappers for intermediate classes.

• Tests

  • Added tests validating proxy detection, subclass/instance classification, and method-resolution behavior.
  • Updated upstream test expectations: removed several expected failures and added one new xfail.

Walkthrough

Replaces subclass-based proxy detection with a metaclass-driven approach (ProxyBaseMeta) that treats types exposing _cpu_class as proxies; updates ProxyBase to use the metaclass and prevents generating GPU wrappers for intermediate subclasses; adds tests for proxy detection and MRO/subclassing semantics.

Changes

Cohort / File(s)	Summary
Proxy classification & metaclass python/cuml/cuml/accel/estimator_proxy.py	Introduce ProxyBaseMeta (subclassing abc.ABCMeta), switch proxy detection to check metaclass membership and _cpu_class presence (replace issubclass(..., ProxyBase) usage), make ProxyBase use ProxyBaseMeta, and adjust ArrayAPIProxyBase.__init_subclass__ to only generate _gpu_class wrappers when _cpu_class_path is defined.
Tests for proxy semantics python/cuml/cuml_accel_tests/test_estimator_proxy.py	Add parametrized tests that verify original proxy-capable estimator classes and their fitted instances are recognized as proxies while dynamic subclasses are not; assert MRO contains the underlying CPU class but not the proxy base, and verify subclass/instance semantics.
xfail list update python/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-list.yaml	Remove several previously xfailed sklearn tests and add one new xfail entry (test_grid_search_score_method).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• Several ensemble scikit-learn compatibility improvements #8023 — Modifies python/cuml/cuml/accel/estimator_proxy.py and the ProxyBase implementation (related changes to proxy handling and unpickling/_reconstruct_from_cpu).

Suggested labels

bug

Suggested reviewers

• csadorf
• dantegd
• hcho3

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.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 'Support subclassing proxy estimators' directly and clearly describes the main change: adding support for subclassing proxy estimators through a new metaclass implementation.
Description check	✅ Passed	The description explains the purpose (supporting subclassing), the mechanism (ProxyBaseMeta metaclass), and the rationale (subclasses should not be accelerated as they may rely on internals), all relevant to the changeset.
Linked Issues check	✅ Passed	The PR directly addresses issue #7971 by implementing the ProxyBaseMeta metaclass to prevent imblearn import failures when cuml.accel is active, enabling subclasses of proxied estimators to work correctly.
Out of Scope Changes check	✅ Passed	All changes are in-scope: estimator_proxy.py introduces ProxyBaseMeta and updates ProxyBase; tests verify the new behavior; xfail-list updates reflect test status changes from the metaclass implementation.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Review rate limit: 9/10 reviews remaining, refill in 6 minutes.}

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

python/cuml/cuml_accel_tests/test_estimator_proxy.py (1)

891-923: ⚡ Quick win

Please cover the _parameter_constraints mutation path directly.

This test proves the new MRO/type semantics, but it never exercises the actual breakage from #7971: a downstream subclass copying or updating sklearn class metadata inherited from the proxy. A small follow-up assertion that Sub._parameter_constraints can be copied/updated without hitting the proxy descriptor would turn this into a direct regression test for the imblearn import failure.

As per coding guidelines "Update unit tests when making code changes".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/cuml/cuml_accel_tests/test_estimator_proxy.py` around lines 891 - 923,
Add a targeted assertion that exercises the _parameter_constraints mutation
path: after defining Sub(Base) and creating sub_model, attempt to copy/update
Sub._parameter_constraints (e.g., assign a shallow copy or update dict/list) and
assert it succeeds without triggering the proxy descriptor; reference the
existing symbols Sub, Base, and _parameter_constraints and ensure the test
checks that modifying Sub._parameter_constraints does not raise and results in
the expected mutated value (so the new assertion directly reproduces the
mutation path exercised in the imblearn import failure).

🤖 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/accel/estimator_proxy.py`:
- Around line 135-144: The metaclass ProxyBaseMeta.__new__ omits **kwargs
causing subclass declarations with metaclass keyword args to raise TypeError;
update the __new__ method signature to def __new__(cls, name, bases, ns,
**kwargs) and forward those kwargs to super().__new__(cls, name, bases, ns,
**kwargs), keeping the existing logic that replaces base classes via
getattr(base, "_cpu_class", base) when isinstance(base, ProxyBaseMeta) so
composition with other metaclasses (and their __init_subclass__ handling) works
correctly.

---

Nitpick comments:
In `@python/cuml/cuml_accel_tests/test_estimator_proxy.py`:
- Around line 891-923: Add a targeted assertion that exercises the
_parameter_constraints mutation path: after defining Sub(Base) and creating
sub_model, attempt to copy/update Sub._parameter_constraints (e.g., assign a
shallow copy or update dict/list) and assert it succeeds without triggering the
proxy descriptor; reference the existing symbols Sub, Base, and
_parameter_constraints and ensure the test checks that modifying
Sub._parameter_constraints does not raise and results in the expected mutated
value (so the new assertion directly reproduces the mutation path exercised in
the imblearn import failure).

🪄 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: e2b36d58-6990-4c5c-baf5-a82692d7a20e

📥 Commits

Reviewing files that changed from the base of the PR and between 016840c and 255fdf2.

📒 Files selected for processing (2)

• python/cuml/cuml/accel/estimator_proxy.py
• python/cuml/cuml_accel_tests/test_estimator_proxy.py

[csadorf]

LGTM! Thanks for working through this!

[jcrist]

/merge