diff --git a/docs/changes.rst b/docs/changes.rst index 4916778c..350b7ea1 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -11,6 +11,15 @@ skops Changelog v0.17 ----- +- Fix a regression since v0.12.0 where saving an object whose ``__reduce__`` + raises failed at dump time. ``__reduce__`` is called on every object to + detect a plain constructor call, but Cython extension types with a + ``__cinit__`` and no ``__reduce__`` raise instead of returning one. pandas' + ``BlockValuesRefs`` is such a type and every pandas ``Index`` except + ``RangeIndex`` holds one, so objects containing such an index, or a + ``Series`` or ``DataFrame`` using one, could not be saved. Such objects are + now saved through ``__getstate__``/``__dict__`` again, as before v0.12.0. + :pr:`550` by `Adrin Jalali`_. - Restore the ``skops`` command line entry point. It was declared in ``setup.py`` and lost when the packaging moved to ``pyproject.toml`` in v0.11.0, so ``skops convert`` and ``skops update`` had not been available diff --git a/skops/io/_general.py b/skops/io/_general.py index ef557ec6..097538da 100644 --- a/skops/io/_general.py +++ b/skops/io/_general.py @@ -410,8 +410,16 @@ def object_get_state(obj: Any, save_context: SaveContext) -> dict[str, Any]: # ``datetime.timezone`` for instance returns ``(timezone, (offset,), None)``. # If the constructor is the same as the object's type, then we consider it # safe to call it with the specified arguments. - - reduce_output = obj.__reduce__() + # + # The call is only a probe for that shape. Objects that cannot be pickled + # raise from ``__reduce__``, e.g. Cython extension types with a + # ``__cinit__`` such as ``pandas._libs.internals.BlockValuesRefs``, and for + # those we fall through to the ``__getstate__``/``__dict__`` path below, as + # we did before this probe existed. + try: + reduce_output = obj.__reduce__() + except Exception: + reduce_output = () if ( len(reduce_output) >= 2 and reduce_output[0] is type(obj) diff --git a/skops/io/tests/test_persist.py b/skops/io/tests/test_persist.py index 387a1b1f..30adf0e8 100644 --- a/skops/io/tests/test_persist.py +++ b/skops/io/tests/test_persist.py @@ -1520,6 +1520,28 @@ def test_custom_reduce(): assert obj.value == loaded_obj.value +# This class is here as opposed to inside the test because it needs to be importable. +# It mimics Cython extension types with a ``__cinit__`` and no ``__reduce__``, +# such as ``pandas._libs.internals.BlockValuesRefs``, whose ``__reduce__`` +# raises instead of returning a value. +class RaisingReduce: + def __init__(self): + self.x = 3 + + def __reduce__(self): + raise TypeError("no default __reduce__ due to non-trivial __cinit__") + + +def test_reduce_raises_falls_back_to_dict(): + # ``__reduce__`` is only called to probe for a constructor call; objects + # whose ``__reduce__`` raises must still be persisted through ``__dict__``, + # as they were before the probe was added, see gh-450. + dumped = dumps(RaisingReduce()) + loaded_obj = loads(dumped, trusted=[RaisingReduce]) + assert type(loaded_obj) is RaisingReduce + assert loaded_obj.x == 3 + + def test_loss_get_state_unsupported_reduce(): # loss_get_state understands the two shapes of __reduce__ output produced by # scikit-learn's loss classes, and refuses anything else.