Skip to content

Commit

Permalink
Process: Ensure that the raw inputs are not mutated (#251)
Browse files Browse the repository at this point in the history
The `Process.raw_inputs` property returns an `AttributesFrozenDict`
instance with the inputs that were originally passed to the constructor
of the process instance. These inputs should not be mutated as the final
inputs are generated from it by pre-processing them with respect to the
process spec, filling in defaults for missing ports.

However, the `spec().inputs.pre_process` call in `on_create` was passing
a shallow copy of the `raw_inputs` and so nested dictionaries would get
modified.

The problem is fixed by passing in a deep copy instead. This is done
using a custom inline function `recursively_copy_dictionaries` instead
of the maybe more obvious choice `copy.deepcopy`. The reason is that the
latter not only copies the namespaces but also the values, which is not
what we want here because we want to maintain the original values.
  • Loading branch information
sphuber authored Nov 29, 2022
1 parent 7a62df5 commit 4701ad3
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 2 deletions.
14 changes: 12 additions & 2 deletions src/plumpy/processes.py
Original file line number Diff line number Diff line change
Expand Up @@ -731,8 +731,18 @@ def on_create(self) -> None:
"""Entering the CREATED state."""
self._creation_time = time.time()

# This will parse the inputs with respect to the input portnamespace of the spec and validate them
raw_inputs = dict(self._raw_inputs) if self._raw_inputs else {}
def recursively_copy_dictionaries(value):
"""Recursively copy the mapping but only create copies of the dictionaries not the values."""
if isinstance(value, dict):
return {key: recursively_copy_dictionaries(subvalue) for key, subvalue in value.items()}
return value

# This will parse the inputs with respect to the input portnamespace of the spec and validate them. The
# ``pre_process`` method of the inputs port namespace modifies its argument in place, and since the
# ``_raw_inputs`` should not be modified, we pass a clone of it. Note that we only need a clone of the nested
# dictionaries, so we don't use ``copy.deepcopy`` (which might seem like the obvious choice) as that will also
# create a clone of the values, which we don't want.
raw_inputs = recursively_copy_dictionaries(dict(self._raw_inputs)) if self._raw_inputs else {}
self._parsed_inputs = self.spec().inputs.pre_process(raw_inputs)
result = self.spec().inputs.validate(self._parsed_inputs)

Expand Down
22 changes: 22 additions & 0 deletions test/test_processes.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,28 @@ def define(cls, spec):
with self.assertRaises(AttributeError):
p.raw_inputs.b

def test_raw_inputs(self):
"""Test that the ``raw_inputs`` are not mutated by the ``Process`` constructor.
Regression test for https://github.com/aiidateam/plumpy/issues/250
"""

class Proc(Process):

@classmethod
def define(cls, spec):
super().define(spec)
spec.input('a')
spec.input('nested.a')
spec.input('nested.b', default='default-value')

inputs = {'a': 5, 'nested': {'a': 'value'}}
process = Proc(inputs)

# Compare against a clone of the original inputs dictionary as the original is modified. It should not contain
# the default value of the ``nested.b`` port.
self.assertDictEqual(dict(process.raw_inputs), {'a': 5, 'nested': {'a': 'value'}})

def test_inputs_default(self):

class Proc(utils.DummyProcess):
Expand Down

0 comments on commit 4701ad3

Please sign in to comment.