Skip to content

Commit

Permalink
Fix issues due to the Parameterized private namespace (#790)
Browse files Browse the repository at this point in the history
  • Loading branch information
maximlt authored Jul 21, 2023
1 parent bcf9a74 commit 524bf57
Show file tree
Hide file tree
Showing 4 changed files with 192 additions and 30 deletions.
35 changes: 25 additions & 10 deletions param/parameterized.py
Original file line number Diff line number Diff line change
Expand Up @@ -848,15 +848,6 @@ def __new__(cls_, *args, **kwargs):
values['precedence'] = 0
return super().__new__(cls_, **values)

def __iter__(self):
"""
Backward compatibility layer to allow tuple unpacking without
the precedence value. Important for Panel which creates a
custom Watcher and uses tuple unpacking. Will be dropped in
Param 3.x.
"""
return iter(self[:-1])

def __str__(self):
cls = type(self)
attrs = ', '.join([f'{f}={getattr(self, f)!r}' for f in cls._fields])
Expand Down Expand Up @@ -1415,6 +1406,9 @@ def __set__(self, obj, val):
_old = self.default
self.default = val
else:
# When setting a Parameter before calling super.
if not isinstance(obj._param__private, _InstancePrivate):
obj._param__private = _InstancePrivate()
_old = obj._param__private.values.get(self.name, self.default)
obj._param__private.values[self.name] = val

Expand Down Expand Up @@ -3638,6 +3632,7 @@ class _ClassPrivate:
'disable_instance_params',
'renamed',
'params',
'initialized',
]

def __init__(
Expand All @@ -3658,6 +3653,14 @@ def __init__(
self.disable_instance_params = disable_instance_params
self.renamed = renamed
self.params = {} if params is None else params
self.initialized = False

def __getstate__(self):
return {slot: getattr(self, slot) for slot in self.__slots__}

def __setstate__(self, state):
for k, v in state.items():
setattr(self, k, v)


class _InstancePrivate:
Expand Down Expand Up @@ -3710,6 +3713,13 @@ def __init__(
# self.watchers = {} if watchers is None else watchers
self.values = {} if values is None else values

def __getstate__(self):
return {slot: getattr(self, slot) for slot in self.__slots__}

def __setstate__(self, state):
for k, v in state.items():
setattr(self, k, v)


class Parameterized(metaclass=ParameterizedMetaclass):
"""
Expand Down Expand Up @@ -3760,7 +3770,12 @@ class Foo(Parameterized):
def __init__(self, **params):
global object_count

self._param__private = _InstancePrivate()
# Setting a Parameter value in an __init__ block before calling
# Parameterized.__init__ (via super() generally) already sets the
# _InstancePrivate namespace over the _ClassPrivate namespace
# (see Parameter.__set__) so we shouldn't override it here.
if not isinstance(self._param__private, _InstancePrivate):
self._param__private = _InstancePrivate()
self._param_watchers = {}

# Skip generating a custom instance name when a class in the hierarchy
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ python_files = "test*.py"
filterwarnings = [
"error",
]
xfail_strict = "true"

[tool.coverage.report]
omit = ["param/version.py"]
136 changes: 136 additions & 0 deletions tests/testparameterizedobject.py
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,142 @@ def test_remove_class_param_validation(self):
with self.assertRaises(ValueError):
TestPOValidation.value = 10

def test_instantiation_set_before_super(self):
count = 0
class P(param.Parameterized):

x = param.Parameter(0)

def __init__(self, x=1):
self.x = x
super().__init__()

@param.depends('x', watch=True)
def cb(self):
nonlocal count
count += 1

p = P()

assert p.x == 1
assert count == 0

def test_instantiation_set_before_super_contrived(self):
# https://github.com/holoviz/param/pull/790#discussion_r1263483293
class P(param.Parameterized):

value = param.String(default="A")

def __init__(self, depth=0):
self.value = 'B'
if depth < 2:
self.sub = P(depth+1)
super().__init__()

p = P()

assert p.value == 'B'
assert p.sub.value == 'B'

def test_instantiation_set_before_super_subclass(self):
# Inspired by a HoloViews use case (GenericElementPlot, GenericOverlayPlot)
class A(param.Parameterized):

def __init__(self, batched=False, **params):
self.batched = batched
super().__init__(**params)

class B(A):

batched = param.Boolean()

def __init__(self, batched=True, **params):
super().__init__(batched=batched, **params)

a = A()
assert a.batched is False

# When b is instantiated the `batched` Parameter of B is set before
# Parameterized.__init__ is called.
b = B()
assert b.batched is True

def test_instantiation_param_objects_before_super_subclass(self):
# Testing https://github.com/holoviz/param/pull/420


class P(param.Parameterized):
x = param.Parameter()

def __init__(self):
objs = self.param.objects(instance='existing')
assert isinstance(objs, dict)
super().__init__()

P()

@pytest.mark.xfail(
raises=AttributeError,
reason='Behavior not defined when setting a constant parameter before calling super()',
)
def test_instantiation_set_before_super_constant(self):
count = 0
class P(param.Parameterized):

x = param.Parameter(0, constant=True)

def __init__(self, x=1):
self.x = x
super().__init__()

@param.depends('x', watch=True)
def cb(self):
nonlocal count
count += 1

p = P()

assert p.x == 1
assert count == 0

def test_instantiation_set_before_super_readonly(self):
class P(param.Parameterized):

x = param.Parameter(0, readonly=True)

def __init__(self, x=1):
self.x = x
super().__init__()

with pytest.raises(TypeError, match="Read-only parameter 'x' cannot be modified"):
P()

def test_parameter_constant_iadd_allowed(self):
# Testing https://github.com/holoviz/param/pull/400
class P(param.Parameterized):

list = param.List([], constant=True)

p = P()
p.list += [1, 2, 3]

# Just to make sure that normal setting is still forbidden
with pytest.raises(TypeError, match="Constant parameter 'list' cannot be modified"):
p.list = [0]

def test_parameter_constant_same_notallowed(self):
L = [0, 1]
class P(param.Parameterized):

list = param.List(L, constant=True)

p = P()

# instantiate is set to true internally so a deepcopy is made of L,
# it's no longer the same object
with pytest.raises(TypeError, match="Constant parameter 'list' cannot be modified"):
p.list = L

def test_values(self):
"""Basic tests of params() method."""

Expand Down
50 changes: 30 additions & 20 deletions tests/testpickle.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,24 +38,27 @@ class P1(param.Parameterized):
x = param.Parameter()

@pytest.mark.parametrize('pickler', [cloudpickle, pickle], indirect=True)
def test_pickle_simple_class(pickler):
s = pickler.dumps(P1)
@pytest.mark.parametrize('protocol', [0, pickle.DEFAULT_PROTOCOL, pickle.HIGHEST_PROTOCOL])
def test_pickle_simple_class(pickler, protocol):
s = pickler.dumps(P1, protocol=protocol)
cls = pickler.loads(s)
assert cls is P1


@pytest.mark.parametrize('pickler', [cloudpickle, pickle], indirect=True)
def test_pickle_simple_instance(pickler):
@pytest.mark.parametrize('protocol', [0, pickle.DEFAULT_PROTOCOL, pickle.HIGHEST_PROTOCOL])
def test_pickle_simple_instance(pickler, protocol):
p = P1()
s = pickler.dumps(p)
s = pickler.dumps(p, protocol=protocol)
inst = pickler.loads(s)
assert eq(p, inst)


@pytest.mark.parametrize('pickler', [cloudpickle, pickle], indirect=True)
def test_pickle_simple_instance_modif_after(pickler):
@pytest.mark.parametrize('protocol', [0, pickle.DEFAULT_PROTOCOL, pickle.HIGHEST_PROTOCOL])
def test_pickle_simple_instance_modif_after(pickler, protocol):
p = P1()
s = pickler.dumps(p)
s = pickler.dumps(p, protocol=protocol)
p.x = 'modified'
inst = pickler.loads(s)
assert not eq(p, inst)
Expand Down Expand Up @@ -97,16 +100,18 @@ class P2(param.Parameterized):


@pytest.mark.parametrize('pickler', [cloudpickle, pickle], indirect=True)
def test_pickle_all_parameters_class(pickler):
s = pickler.dumps(P2)
@pytest.mark.parametrize('protocol', [0, pickle.DEFAULT_PROTOCOL, pickle.HIGHEST_PROTOCOL])
def test_pickle_all_parameters_class(pickler, protocol):
s = pickler.dumps(P2, protocol=protocol)
cls = pickler.loads(s)
assert cls is P2


@pytest.mark.parametrize('pickler', [cloudpickle, pickle], indirect=True)
def test_pickle_all_parameters_instance(pickler):
@pytest.mark.parametrize('protocol', [0, pickle.DEFAULT_PROTOCOL, pickle.HIGHEST_PROTOCOL])
def test_pickle_all_parameters_instance(pickler, protocol):
p = P2()
s = pickler.dumps(p)
s = pickler.dumps(p, protocol=protocol)
inst = pickler.loads(s)
assert eq(p, inst)

Expand All @@ -121,17 +126,19 @@ def cb(self):


@pytest.mark.parametrize('pickler', [cloudpickle, pickle], indirect=True)
def test_pickle_depends_watch_class(pickler):
s = pickler.dumps(P3)
@pytest.mark.parametrize('protocol', [0, pickle.DEFAULT_PROTOCOL, pickle.HIGHEST_PROTOCOL])
def test_pickle_depends_watch_class(pickler, protocol):
s = pickler.dumps(P3, protocol=protocol)
cls = pickler.loads(s)
assert cls is P3


@pytest.mark.parametrize('pickler', [cloudpickle, pickle], indirect=True)
def test_pickle_depends_watch_instance(pickler):
@pytest.mark.parametrize('protocol', [0, pickle.DEFAULT_PROTOCOL, pickle.HIGHEST_PROTOCOL])
def test_pickle_depends_watch_instance(pickler, protocol):
# https://github.com/holoviz/param/issues/757
p = P3()
s = pickler.dumps(p)
s = pickler.dumps(p, protocol=protocol)
inst = pickler.loads(s)
assert eq(p, inst)

Expand Down Expand Up @@ -176,27 +183,30 @@ def nested(self):


@pytest.mark.parametrize('pickler', [cloudpickle, pickle], indirect=True)
def test_pickle_complex_depends_class(pickler):
s = pickler.dumps(P4)
@pytest.mark.parametrize('protocol', [0, pickle.DEFAULT_PROTOCOL, pickle.HIGHEST_PROTOCOL])
def test_pickle_complex_depends_class(pickler, protocol):
s = pickler.dumps(P4, protocol=protocol)
cls = pickler.loads(s)
assert cls is P4


@pytest.mark.parametrize('pickler', [cloudpickle, pickle], indirect=True)
def test_pickle_complex_depends_instance(pickler):
@pytest.mark.parametrize('protocol', [0, pickle.DEFAULT_PROTOCOL, pickle.HIGHEST_PROTOCOL])
def test_pickle_complex_depends_instance(pickler, protocol):
p = P4()
s = pickler.dumps(p)
s = pickler.dumps(p, protocol=protocol)
inst = pickler.loads(s)
assert eq(p, inst)


@pytest.mark.skipif(cloudpickle is None, reason='cloudpickle not available')
def test_issue_757():
@pytest.mark.parametrize('protocol', [0, pickle.DEFAULT_PROTOCOL, pickle.HIGHEST_PROTOCOL])
def test_issue_757(protocol):
# https://github.com/holoviz/param/issues/759
class P(param.Parameterized):
a = param.Parameter()

p = P()
s = cloudpickle.dumps(p)
s = cloudpickle.dumps(p, protocol=protocol)
inst = cloudpickle.loads(s)
assert eq(p, inst)

0 comments on commit 524bf57

Please sign in to comment.