diff --git a/skbase/base/_base.py b/skbase/base/_base.py index 173c7fd9..926b0158 100644 --- a/skbase/base/_base.py +++ b/skbase/base/_base.py @@ -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 @@ -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(): diff --git a/skbase/tests/test_base.py b/skbase/tests/test_base.py index 33e48aec..4483d19b 100644 --- a/skbase/tests/test_base.py +++ b/skbase/tests/test_base.py @@ -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