diff --git a/param/parameterized.py b/param/parameterized.py index 2e2444b3..6c3ebded 100644 --- a/param/parameterized.py +++ b/param/parameterized.py @@ -1855,7 +1855,7 @@ def __setattr__(self, attribute: str, value): pass super().__setattr__(attribute, value) - if is_slot and attribute in _PARAMETER_CACHE_ATTRS: + if is_slot and attribute in _PARAMETER_CACHE_ATTRS and value is not old: self._invalidate_init_cache() if has_watcher and old is not NotImplemented: self._trigger_event(attribute, old, value) @@ -2411,6 +2411,21 @@ def __exit__(self, exc_type, exc_value, exc_tb): self._restore = {} +def _find_descriptor(cls: type, attr: str) -> t.Any: + """ + Return the descriptor implementing ``attr`` on ``cls``, or None. + + Used to invoke a descriptor without going through attribute lookup on the + object, since an AttributeError raised inside a descriptor is + indistinguishable from a missing attribute to the attribute machinery, + which clears it and dispatches to ``__getattr__``. + """ + for klass in cls.__mro__: + if attr in klass.__dict__: + return klass.__dict__[attr] + return None + + class Parameters: """ Object that holds the ``.param`` namespace and implementation of @@ -2562,9 +2577,26 @@ def __getattr__(self_, attr: str) -> t.Any: if cls is None: # Class not initialized raise AttributeError - if attr in self_._cls_parameters: + ns_type = type(self_) + + # The cached parameters are read from _param__private directly rather + # than via the _cls_parameters property. If that property raised an + # AttributeError we would be called to handle it and recurse, so it is + # only invoked as a descriptor, i.e. without attribute lookup on self_. + params = cls._param__private.params + if not params: + params = _find_descriptor(ns_type, '_cls_parameters').__get__(self_, ns_type) + if attr in params: return self_.__getitem__(attr) - elif self_.self is None: + + # attr is not a Parameter, so if it does exist on this class the + # AttributeError we are handling was raised inside its descriptor. + # Invoking it again surfaces that error instead of masking it. + descriptor = _find_descriptor(ns_type, attr) + if descriptor is not None: + return descriptor.__get__(self_, ns_type) + + if self_.self is None: raise AttributeError(f"type object '{self_.cls.__name__}.param' has no attribute {attr!r}") else: raise AttributeError(f"'{self_.cls.__name__}.param' object has no attribute {attr!r}") @@ -3140,18 +3172,21 @@ def _cls_parameters(self_) -> dict[str, Parameter]: pdict = private.params if pdict: if private.params_to_deepcopy is None or private.params_to_ref is None or private.params_with_default_factory is None: - private.params_to_deepcopy = [] - private.params_to_ref = [] - private.params_with_default_factory = [] + to_deepcopy = [] + to_ref = [] + with_default_factory = [] for pname, pobj in pdict.items(): if pname == 'name': continue if pobj.default_factory is not None: - private.params_with_default_factory.append((pname, pobj)) + with_default_factory.append((pname, pobj)) elif pobj.instantiate: - private.params_to_deepcopy.append(pobj) + to_deepcopy.append(pobj) elif pobj.constant: - private.params_to_ref.append(pobj) + to_ref.append(pobj) + private.params_to_deepcopy = to_deepcopy + private.params_to_ref = to_ref + private.params_with_default_factory = with_default_factory return pdict paramdict = {} diff --git a/tests/testparameterizedobject.py b/tests/testparameterizedobject.py index 5abb5132..7e5cff89 100644 --- a/tests/testparameterizedobject.py +++ b/tests/testparameterizedobject.py @@ -3,6 +3,7 @@ import inspect import re import sys +import threading import unittest import warnings import weakref @@ -2002,3 +2003,108 @@ class P(param.Parameterized): del obj assert freed, "Parameterized instance not freed immediately — likely a reference cycle via .param" + + +def test_no_op_slot_set_does_not_invalidate_init_cache(): + class P(param.Parameterized): + x = param.Number(1) + + P.param.objects('existing') + private = P._param__private + assert private.params_to_deepcopy is not None + + P.param.x.constant = P.param.x.constant + P.param.x.instantiate = P.param.x.instantiate + assert private.params_to_deepcopy is not None + + P.param.x.constant = not P.param.x.constant + assert private.params_to_deepcopy is None + + +def test_cls_parameters_rebuild_survives_concurrent_invalidation(): + # Setting constant/instantiate/default_factory on a Parameter invalidates + # the init caches. If that happens while _cls_parameters is rebuilding + # them, the rebuild must not fail (it used to append to None). + class Invalidating(param.Parameter): + + def __getattribute__(self, key): + if key == 'instantiate': + try: + owner = object.__getattribute__(self, 'owner') + except AttributeError: + owner = None + if owner is not None: + private = owner._param__private + private.params_to_deepcopy = None + private.params_to_ref = None + private.params_with_default_factory = None + return super().__getattribute__(key) + + class P(param.Parameterized): + a = Invalidating() + b = param.String(constant=True) + + assert set(P.param.objects('existing')) == {'name', 'a', 'b'} + + # Only invalidate the init caches, so that the rebuild happens in the + # branch that reuses the already cached parameters + private = P._param__private + assert private.params + private.params_to_deepcopy = None + private.params_to_ref = None + private.params_with_default_factory = None + + assert set(P.param.objects('existing')) == {'name', 'a', 'b'} + + +def test_cls_parameters_rebuild_is_thread_safe(): + class P(param.Parameterized): + pass + + for i in range(100): + P.param.add_parameter(f'p{i}', param.Integer(default=i, constant=bool(i % 2))) + + p = P() + errors = [] + stop = threading.Event() + + def toggle_readonly(): + # Mimics panel.util.parameters.edit_readonly + try: + for _ in range(500): + if stop.is_set(): + return + params = list(p.param.objects('existing').values()) + constants = [po.constant for po in params] + for po in params: + po.constant = False + for po, constant in zip(params, constants): + po.constant = constant + except Exception as e: + errors.append(e) + stop.set() + + threads = [threading.Thread(target=toggle_readonly) for _ in range(4)] + for t in threads: + t.start() + for t in threads: + t.join() + + assert not errors, errors[0] + + +def test_param_namespace_getattr_does_not_mask_descriptor_errors(): + class P(param.Parameterized): + x = param.Number(1) + + original = parameterized.Parameters._cls_parameters + + def broken(self_): + raise AttributeError('the real error') + + try: + parameterized.Parameters._cls_parameters = property(broken) + with pytest.raises(AttributeError, match='the real error'): + P().param.objects('existing') + finally: + parameterized.Parameters._cls_parameters = original