Fix masked dtype equality in cudf.pandas so real_dtype == proxy_dtype holds - #22842
Conversation
|
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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughProxy pandas dtypes in cuDF now expose the matching pandas dtype class via ChangesDtype Proxy Equality Compatibility
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/merge |
… holds (NVIDIA#22842) Fixes the `test_reductions_2d_axis0` failures for masked dtypes (`Boolean`/`Int*`/`UInt*`) in the `cudf.pandas` pandas test suite. ### Root cause The test asserts `dtype == expected.dtype`, where: - `dtype` is a **real** pandas dtype pulled from pandas' internal `NUMPY_INT_TO_DTYPE` table, and - `expected.dtype` is a `cudf.pandas` **proxy** dtype. This evaluates `real_dtype.__eq__(proxy_dtype)`. pandas' `ExtensionDtype.__eq__` does `isinstance(other, type(self))`. Since the proxy only subclasses the abstract `ExtensionDtype` (not the concrete `pd.Int64Dtype`), the `isinstance` check fails and equality returns `False` — even though the two dtypes are semantically identical. Note the asymmetry: `proxy == real` already worked (it dispatches through the proxy's own `__eq__`); only `real == proxy` failed. ### Fix Expose the concrete slow type as `__class__` on each masked dtype proxy. `isinstance` consults `__class__`, so `isinstance(proxy, pd.Int64Dtype)` becomes `True`, equality proceeds, and since these masked dtypes have empty `_metadata` (`()`) it reduces to exactly that type check. `type(proxy)` is unaffected (it bypasses `__class__`), so the proxy machinery that keys off the real type is undisturbed. ### Why this approach - **Fix equality on the proxy, not the data.** The `real == proxy` asymmetry occurs anywhere pandas internals hold real dtype instances (`NUMPY_INT_TO_DTYPE`, `cast.py`, the dtype registry, ...) and compare them against proxy-derived dtypes. Fixing the proxy repairs every such site at once; patching individual call sites would be whack-a-mole. - **Proxying `NUMPY_INT_TO_DTYPE` is not viable.** It's a plain dict whose values are real dtype instances built during pandas' own module execution; the module accelerator wraps namespace attributes, not the interior of data containers, so it is (and stays) un-proxied. - **Scoped to dtype proxies only — not a blanket `__class__` spoof.** These dtypes are immutable, have empty `_metadata`, and are never reconstructed via `self.__class__(...)`, so spoofing `__class__` is safe. Doing it globally for stateful proxies (Series/DataFrame/arrays) would be dangerous: reconstruction via `self.__class__(...)` would yield native pandas objects, and proxy detection / copy / pickle all rely on the true class. With equality fixed, all 28 previously-xfailed `test_reductions_2d_axis0` parametrizations now pass, so their entries are removed from `NODEIDS_THAT_FAIL`. Authors: - GALI PREM SAGAR (https://github.com/galipremsagar) Approvers: - Matthew Roeschke (https://github.com/mroeschke) URL: NVIDIA#22842
Description
Fixes the
test_reductions_2d_axis0failures for masked dtypes (Boolean/Int*/UInt*) in thecudf.pandaspandas test suite.Root cause
The test asserts
dtype == expected.dtype, where:dtypeis a real pandas dtype pulled from pandas' internalNUMPY_INT_TO_DTYPEtable, andexpected.dtypeis acudf.pandasproxy dtype.This evaluates
real_dtype.__eq__(proxy_dtype). pandas'ExtensionDtype.__eq__doesisinstance(other, type(self)). Since the proxy only subclasses the abstractExtensionDtype(not the concretepd.Int64Dtype), theisinstancecheck fails and equality returnsFalse— even though the two dtypes are semantically identical. Note the asymmetry:proxy == realalready worked (it dispatches through the proxy's own__eq__); onlyreal == proxyfailed.Fix
Expose the concrete slow type as
__class__on each masked dtype proxy.isinstanceconsults__class__, soisinstance(proxy, pd.Int64Dtype)becomesTrue, equality proceeds, and since these masked dtypes have empty_metadata(()) it reduces to exactly that type check.type(proxy)is unaffected (it bypasses__class__), so the proxy machinery that keys off the real type is undisturbed.Why this approach
real == proxyasymmetry occurs anywhere pandas internals hold real dtype instances (NUMPY_INT_TO_DTYPE,cast.py, the dtype registry, ...) and compare them against proxy-derived dtypes. Fixing the proxy repairs every such site at once; patching individual call sites would be whack-a-mole.NUMPY_INT_TO_DTYPEis not viable. It's a plain dict whose values are real dtype instances built during pandas' own module execution; the module accelerator wraps namespace attributes, not the interior of data containers, so it is (and stays) un-proxied.__class__spoof. These dtypes are immutable, have empty_metadata, and are never reconstructed viaself.__class__(...), so spoofing__class__is safe. Doing it globally for stateful proxies (Series/DataFrame/arrays) would be dangerous: reconstruction viaself.__class__(...)would yield native pandas objects, and proxy detection / copy / pickle all rely on the true class.With equality fixed, all 28 previously-xfailed
test_reductions_2d_axis0parametrizations now pass, so their entries are removed fromNODEIDS_THAT_FAIL.Checklist