From 25261fab84e7bb856ecdcc6c7ddd870fbe649cb9 Mon Sep 17 00:00:00 2001 From: James McCreight Date: Fri, 14 Aug 2026 12:11:04 -0600 Subject: [PATCH] do not mutate caller-supplied metadata in Parameter and Dimension Strict-mode construction aliased the per-entry sub-dict of the supplied metadata (self.meta = meta[name]), so creation-time bounded maximum resolution (Parameter) and the size setter (Dimension) wrote through to the caller's dict. A second Parameters instance built from the same metadata then read a resolved numeric 'maximum' where Parameters.add expects a dimension name, raising on bounded parameters. Copy the sub-dict at construction; instances keep their resolved values, shared metadata stays pristine. Adds a regression test with two instances sharing one MetaData dict. --- pyPRMS/dimensions/Dimension.py | 6 +++++- pyPRMS/parameters/Parameter.py | 6 +++++- tests/func/test_Parameters.py | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) 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