FIX a raising reduce shouldn't make dump fail

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

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)

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.