Skip to content
Open
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
22 changes: 21 additions & 1 deletion skbase/base/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,14 @@ def set_params(self, **params):
return self
valid_params = self.get_params(deep=True)

# snapshot instance state, to restore it if the reset call below raises,
# e.g., through a parameter validation failure in __init__.
# Must be taken before the setattr writes below, since those writes are
# what leave self in a state that __init__ could not have produced.
# This is a shallow copy of the instance __dict__, i.e., it stores
# references to attribute values, it does not copy the values themselves.
prev_state = self.__dict__.copy()

unmatched_keys = []

nested_params = defaultdict(dict) # grouped by prefix
Expand All @@ -405,7 +413,19 @@ def set_params(self, **params):

# all matched params have now been set
# reset object to clean post-init state with those params
self.reset()
try:
self.reset()
except Exception as e:
# restore the pre-call state, then report what happened,
# the original exception is chained via "from e"
self.__dict__.clear()
self.__dict__.update(prev_state)
raise RuntimeError(
f"Error in {type(self).__name__}.set_params, the parameter values "
f"passed were rejected when re-running __init__, which raised "
f"{type(e).__name__}: {e}. The object has been restored to its "
f"state before the set_params call."
) from e

# recurse in components
for key, sub_params in nested_params.items():
Expand Down
48 changes: 48 additions & 0 deletions skbase/tests/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,54 @@ def test_set_params_with_no_param_to_set_returns_object(
)


class ValidatingObject(BaseObject):
"""BaseObject whose __init__ writes then validates a parameter.

Regression fixture for https://github.com/sktime/sktime/issues/10695 :
__init__ assigns ``self.x`` before raising, mirroring the common pattern
of estimators that write a hyper-parameter to ``self`` ahead of a
validation check.
"""

def __init__(self, x=1):
self.x = x
if x < 0:
raise ValueError("x must be non-negative")
super().__init__()


def test_set_params_rolls_back_state_on_invalid_value():
"""Test a failed set_params leaves the object in its pre-call state.

A failed set_params previously left self holding the rejected value,
since set_params writes parameters to self before reset() re-runs
__init__ for validation. A state __init__ could never have produced
would then survive the raise, and break get_params, clone and repeated
set_params calls from that point on.
"""
obj = ValidatingObject(x=5)

with pytest.raises(RuntimeError, match="restored to its state") as exc_info:
obj.set_params(x=-1)

# the exception names the failing object and reports the restore
assert "ValidatingObject.set_params" in str(exc_info.value)
# the original __init__ exception is chained, not discarded
assert isinstance(exc_info.value.__cause__, ValueError)
assert "x must be non-negative" in str(exc_info.value.__cause__)

assert obj.x == 5
assert obj.get_params() == {"x": 5}

# clone should still work off the pre-call state
cloned = obj.clone()
assert cloned.get_params() == {"x": 5}

# a subsequent valid set_params call should still work normally
obj.set_params(x=9)
assert obj.get_params() == {"x": 9}


# This section tests the clone functionality
# These have been adapted from sklearn's tests of clone to use the clone
# method that is included as part of the BaseObject interface
Expand Down