Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues due to the Parameterized private namespace #790

Merged
merged 8 commits into from
Jul 21, 2023
Merged
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
35 changes: 25 additions & 10 deletions param/parameterized.py
Original file line number Diff line number Diff line change
Expand Up @@ -845,15 +845,6 @@ def __new__(cls_, *args, **kwargs):
values['precedence'] = 0
return super().__new__(cls_, **values)

def __iter__(self):
philippjfr marked this conversation as resolved.
Show resolved Hide resolved
"""
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 @@ -1412,6 +1403,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 @@ -3627,6 +3621,7 @@ class _ClassPrivate:
'disable_instance_params',
'renamed',
'params',
'initialized',
]

def __init__(
Expand All @@ -3647,6 +3642,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 @@ -3699,6 +3702,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 @@ -3749,7 +3759,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 @@ -29,24 +29,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 @@ -88,16 +91,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 @@ -112,17 +117,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 @@ -167,27 +174,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)