Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
50 changes: 31 additions & 19 deletions skbase/base/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -384,28 +384,40 @@ def set_params(self, **params):
return self
valid_params = self.get_params(deep=True)

# snapshot instance state, to restore it if setting params below raises,
# e.g., through a validation failure in __init__ during the reset call.
# Without this, a failed set_params can leave self in a state that
# __init__ could not have produced, since setattr writes below happen
# before that validation runs.
prev_state = self.__dict__.copy()

unmatched_keys = []

nested_params = defaultdict(dict) # grouped by prefix
for full_key, value in params.items():
# split full_key by first occurrence of __, if contains __
# "key_without_dblunderscore" -> "key_without_dbl_underscore", None, None
# "key__with__dblunderscore" -> "key", "__", "with__dblunderscore"
key, delim, sub_key = full_key.partition("__")
# if key not recognized, remember for suffix matching
if key not in valid_params:
unmatched_keys += [key]
# if full_key contained __, collect suffix for component set_params
elif delim:
nested_params[key][sub_key] = value
# if key is found and did not contain __, set self.key to the value
else:
setattr(self, key, value)
valid_params[key] = value

# all matched params have now been set
# reset object to clean post-init state with those params
self.reset()
try:
for full_key, value in params.items():
# split full_key by first occurrence of __, if contains __
# "key_without_dblunderscore" -> "key_without_dbl_underscore", None, None
# "key__with__dblunderscore" -> "key", "__", "with__dblunderscore"
key, delim, sub_key = full_key.partition("__")
# if key not recognized, remember for suffix matching
if key not in valid_params:
unmatched_keys += [key]
# if full_key contained __, collect suffix for component set_params
elif delim:
nested_params[key][sub_key] = value
# if key is found and did not contain __, set self.key to the value
else:
setattr(self, key, value)
valid_params[key] = value

# all matched params have now been set
# reset object to clean post-init state with those params
self.reset()
except Exception:
self.__dict__.clear()
self.__dict__.update(prev_state)
raise

# recurse in components
for key, sub_params in nested_params.items():
Expand Down
42 changes: 42 additions & 0 deletions skbase/tests/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,48 @@ 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(ValueError, match="x must be non-negative"):
obj.set_params(x=-1)

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