diff --git a/pyPRMS/dimensions/Dimension.py b/pyPRMS/dimensions/Dimension.py index ea9be4e..e259d45 100644 --- a/pyPRMS/dimensions/Dimension.py +++ b/pyPRMS/dimensions/Dimension.py @@ -35,7 +35,11 @@ def __init__(self, name: str, else: if strict: if name in meta: - self.meta = meta[name] + # Copy the entry so instance modifications (the size + # setter writes self.meta['size']) do not mutate the + # caller-supplied metadata dict, which may be shared + # across instances. + self.meta = dict(meta[name]) else: raise ValueError(f'`{self.name}` does not exist in metadata') else: diff --git a/pyPRMS/parameters/Parameter.py b/pyPRMS/parameters/Parameter.py index eb5f74c..a3259a1 100644 --- a/pyPRMS/parameters/Parameter.py +++ b/pyPRMS/parameters/Parameter.py @@ -69,7 +69,11 @@ def __init__(self, name: str, else: if strict: if name in meta: - self.meta = meta[name] + # Copy the entry so creation-time modifications (e.g. the + # bounded maximum resolution below) do not mutate the + # caller-supplied metadata dict, which may be shared + # across Parameters instances. + self.meta = dict(meta[name]) # Add the dimensions for this parameter for cname in self.meta['dimensions']: diff --git a/tests/func/test_Parameters.py b/tests/func/test_Parameters.py index 5d0cb1f..0c0fcb1 100644 --- a/tests/func/test_Parameters.py +++ b/tests/func/test_Parameters.py @@ -171,3 +171,35 @@ def test_add_adhoc_parameter_metadata(self, pdb_instance): pdb_instance.add('foo') assert pdb_instance.get('foo').__str__() == expected + + +class TestParametersSharedMetadata: + + def test_shared_metadata_not_mutated(self): + """Creating parameters and dimensions must not modify the supplied + metadata dictionary, which may be shared across instances. + + Regression test: bounded-maximum resolution (Parameter) and the + dimension size setter (Dimension) wrote through to the caller's + metadata, so a second Parameters instance built from the same + metadata dict saw a numeric 'maximum' where a dimension name was + expected and raised on add() of a bounded parameter. + """ + prms_meta = MetaData(verbose=False).metadata + max_before = prms_meta['parameters']['outlet_sta']['maximum'] + nobs_before = dict(prms_meta['dimensions']['nobs']) + + for _ in range(2): # the second iteration crashed before the fix + pdb = Parameters(metadata=prms_meta) + pdb.dimensions.add(name='one', size=1) + pdb.dimensions.add(name='npoigages', size=5) + pdb.dimensions.add(name='nobs', size=5) + pdb.add(name='outlet_sta') + + # the instance sees the resolved bound + assert pdb.get('outlet_sta').meta['maximum'] == 5 + + # the shared metadata is untouched + assert prms_meta['parameters']['outlet_sta']['maximum'] == max_before + assert 'bounded_dimension_name' not in prms_meta['parameters']['outlet_sta'] + assert prms_meta['dimensions']['nobs'] == nobs_before