Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions python/cudf/cudf/pandas/_wrappers/pandas.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
_FastSlowAttribute,
_FunctionProxy,
_maybe_wrap_result,
_setattr_fsproxy_no_mirror,
_State,
_Unusable,
is_proxy_object,
Expand Down Expand Up @@ -1691,8 +1692,10 @@ def _df_query_method(self, *args, local_dict=None, global_dict=None, **kwargs):
)


DataFrame.eval = _df_eval_method
DataFrame.query = _df_query_method
# These custom implementations are installed by cudf.pandas itself and must
# not be mirrored onto (and clobber) the real ``pandas.DataFrame``.
_setattr_fsproxy_no_mirror(DataFrame, "eval", _df_eval_method)
_setattr_fsproxy_no_mirror(DataFrame, "query", _df_query_method)

_JsonReader = make_intermediate_proxy_type(
"_JsonReader",
Expand Down
189 changes: 189 additions & 0 deletions python/cudf/cudf/pandas/fast_slow_proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,11 @@ def _fsproxy_state(self) -> _State:
final_type_map[fast_type] = cls
final_type_map[slow_type] = cls

# Proxy type fully constructed: snapshot its pristine state and, from
# here on, mirror class-level attribute writes (genuine runtime
# monkeypatches) onto the underlying "slow" (real) type.
_enable_fsproxy_mirroring(cls)

return cls


Expand Down Expand Up @@ -511,6 +516,11 @@ def _fsproxy_fast_to_slow(self):
intermediate_type_map[fast_type] = cls
intermediate_type_map[slow_type] = cls

# Proxy type fully constructed: snapshot its pristine state and, from
# here on, mirror class-level attribute writes (genuine runtime
# monkeypatches) onto the underlying "slow" (real) type.
_enable_fsproxy_mirroring(cls)

return cls


Expand Down Expand Up @@ -560,6 +570,55 @@ def get_registered_functions():
return dict()


_SLOW_ABSENT = object()


def _enable_fsproxy_mirroring(cls: type) -> None:
"""Finalize a proxy type for class-level patch mirroring.

Snapshots the proxy type's pristine public class attributes together
with the slow type's pristine class-dict entries for the same names,
then enables mirroring of class-level attribute writes/deletions onto
the slow type (see ``_FastSlowProxyMeta.__setattr__``/``__delattr__``).

The snapshot is a fixed translation table, not runtime patch tracking:
re-assigning the proxy's pristine attribute for ``name`` (which is what
``monkeypatch``/``mock.patch`` save and re-assign on undo) translates
to restoring the slow type's pristine attribute for ``name``.
"""
slow = cls._fsproxy_slow_type # type: ignore[attr-defined]
pristine = {
name: (value, slow.__dict__.get(name, _SLOW_ABSENT))
for name, value in cls.__dict__.items()
if not name.startswith("_")
}
type.__setattr__(cls, "_fsproxy_pristine_attrs", pristine)
type.__setattr__(cls, "_fsproxy_mirror_slow_overrides", True)


def _setattr_fsproxy_no_mirror(cls: type, name: str, value: Any) -> None:
"""Install a cudf.pandas-internal attribute on a proxy type.

``_FastSlowProxyMeta.__setattr__`` mirrors class-level attribute writes
onto the underlying "slow" (real) type so that runtime monkeypatches stay
visible to the pandas fallback path. cudf.pandas itself installs a handful
of custom methods (e.g. ``DataFrame.query``/``DataFrame.eval``) onto the
proxy classes that must *not* clobber pandas' genuine implementations; use
this helper for those (rare) assignments, after the proxy type has been
fully constructed by ``make_*_proxy_type``. The attribute is registered as
part of the proxy's pristine state so that a later save/patch/re-assign
cycle restores the slow type's own attribute rather than forwarding the
cudf-internal object to it.
"""
type.__setattr__(cls, name, value)
if not name.startswith("_"):
slow = cls._fsproxy_slow_type # type: ignore[attr-defined]
cls._fsproxy_pristine_attrs[name] = ( # type: ignore[attr-defined]
value,
slow.__dict__.get(name, _SLOW_ABSENT),
)


class _FastSlowProxyMeta(type):
"""
Metaclass used to dynamically find class attributes and
Expand All @@ -578,6 +637,136 @@ def _fsproxy_slow(self) -> type:
def _fsproxy_fast(self) -> type:
return self._fsproxy_fast_type

def __new__(mcls, *args, **kwargs):
cls = super().__new__(mcls, *args, **kwargs)
# Per-proxy-type switch controlling whether class-level attribute
# writes are mirrored onto the underlying "slow" (real) type (see
# ``__setattr__``/``__delattr__``). It starts disabled so that the
# attributes installed while the proxy type is being built are not
# forwarded to the real type; ``make_*_proxy_type`` enables it once
# construction is complete. Initialized in ``__new__`` rather than
# ``__init__`` because cooperating metaclasses may perform
# class-level attribute writes from their own ``__new__`` — e.g.
# ``ABCMeta.__new__`` assigns ``__abstractmethods__``, dispatching
# to ``__setattr__`` below before ``__init__`` ever runs.
type.__setattr__(cls, "_fsproxy_mirror_slow_overrides", False)
return cls

def __setattr__(cls, name, value):
# Class-level attribute assignments on a proxy type (e.g.
# ``monkeypatch.setattr(pd.ExcelFile, "parse", fn)``) must also be
# mirrored onto the underlying "slow" (real) type. Code that runs
# under ``disable_module_accelerator()`` (e.g. the pandas fallback
# path of ``pd.read_excel``) resolves attributes from the real
# class, not the proxy, so a patch applied only to the proxy would
# otherwise be invisible to that code. The assigned value is first
# translated into its slow-space equivalent: re-assigning the
# proxy's pristine attribute translates to the slow type's pristine
# attribute, and proxy machinery is unwrapped to the slow object it
# delegates to, so save/patch/re-assign cycles round-trip on the
# real type as well.
type.__setattr__(cls, name, value)
if not cls._fsproxy_mirror_slow_overrides:
# The proxy type is still being constructed (or this is a
# non-pandas proxy): only mirror user/runtime monkeypatches,
# never the custom methods cudf.pandas installs on the proxy
# classes itself.
return
if name.startswith("_"):
return
slow = cls._fsproxy_slow_type
try:
# Mirroring is best-effort: translating a wrapped proxy instance
# can require a fast-to-slow conversion, which may itself fail;
# never let that escape an otherwise-successful assignment.
pristine = cls._fsproxy_pristine_attrs
entry = pristine.get(name)
if entry is not None and value is entry[0]:
# The proxy's pristine attribute for ``name`` is being
# re-assigned (e.g. ``monkeypatch``/``mock.patch`` undo
# re-setting the saved class-dict entry). Its slow-space
# equivalent is the slow type's pristine attribute.
if entry[1] is _SLOW_ABSENT:
# The slow type never defined ``name`` itself: the proxy
# mirrors the slow type's *dir*, so it has pristine
# attributes for methods the slow type only inherits
# (e.g. ``DataFrame.head`` lives on ``NDFrame``).
# Mirroring a patch for such a name added a shadowing
# entry to the slow type's dict; undoing the patch must
# remove that entry again so the inherited
# implementation becomes visible. It may legitimately be
# missing (the mirror is best-effort), hence the guard.
if name in slow.__dict__:
delattr(slow, name)
Comment on lines +689 to +700

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When do you hit this path? Shouldn't we avoid ever setting the attribute on the slow type in the first place?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the undo half of a mirror we very much want to make. The proxy's class dict is built from dir(slow_type), so it holds pristine entries for names the slow type only inherits — e.g. DataFrame.head lives on NDFrame, and pandas.DataFrame.__dict__ has no 'head'. When a user patches pd.DataFrame.head, the mirror must set head on pandas.DataFrame itself: that shadowing entry is the only way fallback code resolving through the real class sees the patch. This branch runs when the patch is undone (monkeypatch re-assigns the saved pristine proxy descriptor): the slow-space translation of "restore pristine" for a slow-inherited name is "delete the shadowing entry we added", making the inherited implementation visible again. The name in slow.__dict__ guard covers the case where the original mirror never landed (mirroring is best-effort), so there's nothing to delete. Expanded the code comment with this example — test_class_attr_inherited_method_monkeypatch_roundtrip exercises exactly this cycle.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detailed explanation, this makes sense now.

else:
setattr(slow, name, entry[1])
return
# Otherwise translate the assigned value into "slow" space
# before mirroring it: a value read off a proxy type (e.g. the
# original that a caller saves before patching and re-assigns
# to undo) is proxy machinery wrapping a slow-side object, and
# mirroring it verbatim would install that machinery on the
# real type. Unwrap it to the slow object it delegates to, so
# save/patch/re-assign cycles round-trip on the real type;
# values with no determinable slow-side equivalent are not
# mirrored at all.
if isinstance(value, _FastSlowAttribute):
# The proxy's own delegating descriptor (a *pristine* one is
# already handled by identity above; this covers a
# descriptor obtained some other way): its slow equivalent
# is the method it wraps, if it ever resolved one.
attr = value._attr
if not isinstance(attr, _MethodProxy):
return
value = attr
if isinstance(value, _FunctionProxy):
unwrapped = value._fsproxy_slow
if entry is not None and entry[1] is not _SLOW_ABSENT:
# Re-assigning a saved ``cls.method`` (a ``_MethodProxy``
# over the *resolved* slow attribute): if it resolves
# back to the slow type's pristine attribute, restore
# the pristine class-dict entry itself so
# ``classmethod``/``staticmethod`` descriptors are not
# degraded to their bound/plain-function forms.
descriptor = entry[1]
try:
resolved = (
descriptor.__get__(None, slow)
if hasattr(type(descriptor), "__get__")
else descriptor
)
if unwrapped is resolved or unwrapped == resolved:
setattr(slow, name, descriptor)
return
except Exception:
pass
setattr(slow, name, unwrapped)
elif isinstance(value, _FastSlowProxy):
setattr(slow, name, value._fsproxy_slow)
elif isinstance(value, _FastSlowProxyMeta):
slow_type = getattr(value, "_fsproxy_slow_type", None)
if slow_type is not None:
setattr(slow, name, slow_type)
else:
setattr(slow, name, value)
except Exception:
pass
Comment thread
galipremsagar marked this conversation as resolved.

def __delattr__(cls, name):
# Mirror class-level attribute *deletions* onto the underlying "slow"
# (real) type as well, for the same reason as ``__setattr__``: after
# ``del cls.name`` the attribute is gone from the proxy, so it must
# also be gone from the real type seen by fallback code.
type.__delattr__(cls, name)
if not cls._fsproxy_mirror_slow_overrides:
return
if name.startswith("_"):
return
try:
delattr(cls._fsproxy_slow_type, name)
except (AttributeError, TypeError):
pass

def __dir__(self):
# Try to return the cached dir of the slow object, but if it
# doesn't exist, fall back to the default implementation.
Expand Down
14 changes: 0 additions & 14 deletions python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -2544,19 +2544,6 @@ def pytest_unconfigure(config):
"tests/io/excel/test_odswriter.py::test_cell_value_type[test string-string-string-value-test string]": "TODO: Add a reason for failure",
"tests/io/excel/test_odswriter.py::test_cell_value_type[value4-date-date-value-2010-10-10T10:10:10]": "TODO: Add a reason for failure",
"tests/io/excel/test_odswriter.py::test_cell_value_type[value5-date-date-value-2010-10-10]": "TODO: Add a reason for failure",
"tests/io/excel/test_readers.py::TestReaders::test_engine_used[('calamine', '.ods')]": "TODO: Add a reason for failure",
"tests/io/excel/test_readers.py::TestReaders::test_engine_used[('calamine', '.xls')]": "TODO: Add a reason for failure",
"tests/io/excel/test_readers.py::TestReaders::test_engine_used[('calamine', '.xlsb')]": "TODO: Add a reason for failure",
"tests/io/excel/test_readers.py::TestReaders::test_engine_used[('calamine', '.xlsm')]": "TODO: Add a reason for failure",
"tests/io/excel/test_readers.py::TestReaders::test_engine_used[('calamine', '.xlsx')]": "TODO: Add a reason for failure",
"tests/io/excel/test_readers.py::TestReaders::test_engine_used[('odf', '.ods')]": "TODO: Add a reason for failure",
"tests/io/excel/test_readers.py::TestReaders::test_engine_used[('openpyxl', '.xlsm')]": "TODO: Add a reason for failure",
"tests/io/excel/test_readers.py::TestReaders::test_engine_used[('openpyxl', '.xlsx')]": "TODO: Add a reason for failure",
"tests/io/excel/test_readers.py::TestReaders::test_engine_used[('pyxlsb', '.xlsb')]": "TODO: Add a reason for failure",
"tests/io/excel/test_readers.py::TestReaders::test_engine_used[('xlrd', '.xls')]": "TODO: Add a reason for failure",
"tests/io/excel/test_readers.py::TestReaders::test_engine_used[(None, '.xls')]": "TODO: Add a reason for failure",
"tests/io/excel/test_readers.py::TestReaders::test_engine_used[(None, '.xlsm')]": "TODO: Add a reason for failure",
"tests/io/excel/test_readers.py::TestReaders::test_engine_used[(None, '.xlsx')]": "TODO: Add a reason for failure",
"tests/io/excel/test_style.py::test_format_hierarchical_rows_periodindex[False]": "AttributeError: _compute. Did you mean: 'compare'?",
"tests/io/excel/test_style.py::test_format_hierarchical_rows_periodindex[True]": "AttributeError: _compute. Did you mean: 'compare'?",
"tests/io/excel/test_style.py::test_format_hierarchical_rows_periodindex[columns]": "AttributeError: _compute. Did you mean: 'compare'?",
Expand Down Expand Up @@ -3690,7 +3677,6 @@ def pytest_unconfigure(config):
"tests/test_algos.py::TestValueCounts::test_value_counts_dropna": "pandas keeps bool-with-None data as object dtype; cudf stores it as a masked bool column",
"tests/test_algos.py::TestValueCounts::test_value_counts_stability": "asserts that kind='quicksort' produces an unstable order; cudf sorts are always stable",
"tests/test_col.py::test_cached_property": "AssertionError: assert False",
"tests/test_col.py::test_custom_accessor": "AttributeError: 'Series' object has no attribute 'xyz'",
"tests/test_common.py::test_serializable[obj0]": "TODO: Add a reason for failure",
"tests/test_common.py::test_temp_setattr[False]": "TODO: Add a reason for failure",
"tests/test_common.py::test_temp_setattr[True]": "TODO: Add a reason for failure",
Expand Down
32 changes: 32 additions & 0 deletions python/cudf/cudf_pandas_tests/test_cudf_pandas.py
Original file line number Diff line number Diff line change
Expand Up @@ -2186,6 +2186,38 @@ def test_module_proxy_write_through_config(monkeypatch):
cf.register_option("foo", 1)


def test_class_monkeypatch_roundtrip_restores_real_pandas(monkeypatch):
# Class-level patches on proxy types are mirrored onto the real pandas
# type (so they stay visible to fallback code running under
# ``disable_module_accelerator``); undoing them must restore the real
# type's own attributes — including for attributes that cudf.pandas
# replaces on the proxy, like ``columns``/``eval``/``str``.
real_df = xpd.DataFrame._fsproxy_slow
real_series = xpd.Series._fsproxy_slow
orig_columns = real_df.__dict__["columns"]
orig_eval = real_df.__dict__["eval"]
orig_str = real_series.__dict__["str"]

def fake_eval(self, *args, **kwargs):
return "patched"

monkeypatch.setattr(xpd.DataFrame, "eval", fake_eval)
assert real_df.__dict__["eval"] is fake_eval
monkeypatch.setattr(
xpd.DataFrame, "columns", property(lambda self: "patched")
)
monkeypatch.setattr(xpd.Series, "str", property(lambda self: "patched"))
monkeypatch.undo()

assert real_df.__dict__["columns"] is orig_columns
assert real_df.__dict__["eval"] is orig_eval
assert real_series.__dict__["str"] is orig_str
# The real type must remain fully functional on the fallback path.
df = real_df({"a": [1, 2]})
assert list(df.columns) == ["a"]
assert list(df.eval("b = a + 1").columns) == ["a", "b"]


@pytest.mark.parametrize("box", ["Series", "array"])
@pytest.mark.parametrize("na_value", [pd.NA, np.nan], ids=["NA", "NaN"])
@pytest.mark.parametrize("storage", ["python", "pyarrow"])
Expand Down
Loading
Loading