From 0c2c5e39d4d2ab36088fdbe95cba3b349d2c04cc Mon Sep 17 00:00:00 2001 From: Andy Beer Date: Wed, 24 Jul 2024 17:42:14 +0200 Subject: [PATCH 1/4] warn or exception on duplicate key --- resqpy/property/attribute_property_set.py | 25 ++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/resqpy/property/attribute_property_set.py b/resqpy/property/attribute_property_set.py index 77db9daf..9926fd59 100644 --- a/resqpy/property/attribute_property_set.py +++ b/resqpy/property/attribute_property_set.py @@ -183,7 +183,8 @@ def local_property_kind_uuid(self): class AttributePropertySet(rqp.PropertyCollection): """Class for set of RESQML properties for any supporting representation, using attribute syntax.""" - def __init__(self, model = None, support = None, property_set_uuid = None, realization = None, key_mode = 'pk'): + def __init__(self, model = None, support = None, property_set_uuid = None, realization = None, + key_mode = 'pk', indexable = None, multiple_handling = 'warn'): """Initialise an empty property set, optionally populate properties from a supporting representation. arguments: @@ -198,6 +199,11 @@ def __init__(self, model = None, support = None, property_set_uuid = None, reali if None, then the collection is either covering a whole ensemble (individual properties can each be flagged with a realisation number), or is for properties that do not have multiple realizations key_mode (str, default 'pk'): either 'pk' (for property kind) or 'title', identifying the basis of property attribute keys + indexable (str, optional): if present and key_mode is 'pk', properties with indexable element other than this will + have their indexable element included in their key + multiple_handling (str, default 'warn'): either 'warn' or 'exception'; if warn, and properties exist that generate the + same key, then only the first is visible in the attribute property set (and a warning is given for each of the others); + if exception, a ValueError is raised if there are any duplicate keys note: at present, if the collection is being initialised from a property set, the support argument must also be specified; @@ -214,9 +220,12 @@ def __init__(self, model = None, support = None, property_set_uuid = None, reali property_set_root = None else: property_set_root = model.root_for_uuid(property_set_uuid) + assert multiple_handling in ['warn', 'exception'] super().__init__(support = support, property_set_root = property_set_root, realization = realization) self.key_mode = key_mode + self.indexable_mode = indexable + self.multiple_handling = multiple_handling self._make_attributes() def keys(self): @@ -241,12 +250,19 @@ def _key(self, part): title = self.citation_title_for_part(part), facet = self.facet_for_part(part), time_index = self.time_index_for_part(part), - realization = self.realization_for_part(part)) + realization = self.realization_for_part(part), + indexable_mode = self.indexable_mode, + indexable = self.indexable_for_part(part)) def _make_attributes(self): """Setup individual properties with attribute style read access to metadata.""" for part in self.parts(): key = self._key(part) + if getattr(self, key, None) is not None: + if self.multiple_handling == 'warn': + log.error(f'duplicate key in AttributePropertySet; only first instance included: {key}') + continue + raise ValueError(f'duplicate key in attribute property set: {key}') aps_property = ApsProperty(self, part) setattr(self, key, aps_property) @@ -255,11 +271,14 @@ def __len__(self): return self.number_of_parts() -def make_aps_key(key_mode, property_kind = None, title = None, facet = None, time_index = None, realization = None): +def make_aps_key(key_mode, property_kind = None, title = None, facet = None, time_index = None, + realization = None, indexable_mode = None, indexable = None): """Contructs the key (attribute name) for a property based on metadata items.""" if key_mode == 'pk': assert property_kind is not None key = property_kind + if indexable_mode is not None and indexable is not None and indexable != indexable_mode: + key += f'_{indexable}' if facet is not None: key += f'_{facet}' else: From a70e95569bf15ea43ff6afa0be59288ca7753d0f Mon Sep 17 00:00:00 2001 From: Andy Beer Date: Wed, 24 Jul 2024 17:52:18 +0200 Subject: [PATCH 2/4] downgrading duplicate key from error to warning --- resqpy/property/attribute_property_set.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/resqpy/property/attribute_property_set.py b/resqpy/property/attribute_property_set.py index 9926fd59..b3c38a2d 100644 --- a/resqpy/property/attribute_property_set.py +++ b/resqpy/property/attribute_property_set.py @@ -183,8 +183,14 @@ def local_property_kind_uuid(self): class AttributePropertySet(rqp.PropertyCollection): """Class for set of RESQML properties for any supporting representation, using attribute syntax.""" - def __init__(self, model = None, support = None, property_set_uuid = None, realization = None, - key_mode = 'pk', indexable = None, multiple_handling = 'warn'): + def __init__(self, + model = None, + support = None, + property_set_uuid = None, + realization = None, + key_mode = 'pk', + indexable = None, + multiple_handling = 'warn'): """Initialise an empty property set, optionally populate properties from a supporting representation. arguments: @@ -260,7 +266,7 @@ def _make_attributes(self): key = self._key(part) if getattr(self, key, None) is not None: if self.multiple_handling == 'warn': - log.error(f'duplicate key in AttributePropertySet; only first instance included: {key}') + log.warning(f'duplicate key in AttributePropertySet; only first instance included: {key}') continue raise ValueError(f'duplicate key in attribute property set: {key}') aps_property = ApsProperty(self, part) @@ -271,8 +277,14 @@ def __len__(self): return self.number_of_parts() -def make_aps_key(key_mode, property_kind = None, title = None, facet = None, time_index = None, - realization = None, indexable_mode = None, indexable = None): +def make_aps_key(key_mode, + property_kind = None, + title = None, + facet = None, + time_index = None, + realization = None, + indexable_mode = None, + indexable = None): """Contructs the key (attribute name) for a property based on metadata items.""" if key_mode == 'pk': assert property_kind is not None From 0b54c0311e6dcde18c38b1cfa4119b9b8fd4f22f Mon Sep 17 00:00:00 2001 From: Andy Beer Date: Wed, 24 Jul 2024 17:59:42 +0200 Subject: [PATCH 3/4] ignore option on aps multiple handling --- resqpy/property/attribute_property_set.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/resqpy/property/attribute_property_set.py b/resqpy/property/attribute_property_set.py index b3c38a2d..0a89c5bb 100644 --- a/resqpy/property/attribute_property_set.py +++ b/resqpy/property/attribute_property_set.py @@ -207,9 +207,9 @@ def __init__(self, key_mode (str, default 'pk'): either 'pk' (for property kind) or 'title', identifying the basis of property attribute keys indexable (str, optional): if present and key_mode is 'pk', properties with indexable element other than this will have their indexable element included in their key - multiple_handling (str, default 'warn'): either 'warn' or 'exception'; if warn, and properties exist that generate the - same key, then only the first is visible in the attribute property set (and a warning is given for each of the others); - if exception, a ValueError is raised if there are any duplicate keys + multiple_handling (str, default 'warn'): either 'ignore', 'warn' ,or 'exception'; if 'warn' or 'ignore', and properties + exist that generate the same key, then only the first is visible in the attribute property set (and a warning is given + for each of the others in the case of 'warn'); if 'exception', a ValueError is raised if there are any duplicate keys note: at present, if the collection is being initialised from a property set, the support argument must also be specified; @@ -226,7 +226,7 @@ def __init__(self, property_set_root = None else: property_set_root = model.root_for_uuid(property_set_uuid) - assert multiple_handling in ['warn', 'exception'] + assert multiple_handling in ['ignore', 'warn', 'exception'] super().__init__(support = support, property_set_root = property_set_root, realization = realization) self.key_mode = key_mode @@ -268,6 +268,8 @@ def _make_attributes(self): if self.multiple_handling == 'warn': log.warning(f'duplicate key in AttributePropertySet; only first instance included: {key}') continue + if self.multiple_handling == 'ignore': + continue raise ValueError(f'duplicate key in attribute property set: {key}') aps_property = ApsProperty(self, part) setattr(self, key, aps_property) From a2c657aa5f03257b570a82f082829ac0bb8f43c8 Mon Sep 17 00:00:00 2001 From: Andy Beer Date: Wed, 24 Jul 2024 22:41:14 +0200 Subject: [PATCH 4/4] using KeyError instead of ValueError; unit test added --- resqpy/property/attribute_property_set.py | 4 +- .../property/test_attribute_property_set.py | 51 +++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/resqpy/property/attribute_property_set.py b/resqpy/property/attribute_property_set.py index 0a89c5bb..6f7401d8 100644 --- a/resqpy/property/attribute_property_set.py +++ b/resqpy/property/attribute_property_set.py @@ -209,7 +209,7 @@ def __init__(self, have their indexable element included in their key multiple_handling (str, default 'warn'): either 'ignore', 'warn' ,or 'exception'; if 'warn' or 'ignore', and properties exist that generate the same key, then only the first is visible in the attribute property set (and a warning is given - for each of the others in the case of 'warn'); if 'exception', a ValueError is raised if there are any duplicate keys + for each of the others in the case of 'warn'); if 'exception', a KeyError is raised if there are any duplicate keys note: at present, if the collection is being initialised from a property set, the support argument must also be specified; @@ -270,7 +270,7 @@ def _make_attributes(self): continue if self.multiple_handling == 'ignore': continue - raise ValueError(f'duplicate key in attribute property set: {key}') + raise KeyError(f'duplicate key in attribute property set: {key}') aps_property = ApsProperty(self, part) setattr(self, key, aps_property) diff --git a/tests/unit_tests/property/test_attribute_property_set.py b/tests/unit_tests/property/test_attribute_property_set.py index d918a35d..c32b294c 100644 --- a/tests/unit_tests/property/test_attribute_property_set.py +++ b/tests/unit_tests/property/test_attribute_property_set.py @@ -140,6 +140,57 @@ def test_load_attribute_property_collection_pk(example_model_with_prop_ts_rels): assert np.all(v == aps.saturation_water_t2.array_ref) +def test_load_attribute_property_collection_pk_duplicates(example_model_with_prop_ts_rels, capfd, caplog): + # Arrange + model = example_model_with_prop_ts_rels + grid = model.grid() + pc = grid.extract_property_collection() + + # Act + aps = rqp.AttributePropertySet(support = grid) + facies_col = np.max(aps.facies.values, axis = 0) + pc.add_similar_to_imported_list(similar_uuid = aps.facies.uuid, + cached_array = facies_col, + indexable_element = 'columns') + pc.write_hdf5_for_imported_list() + pc.create_xml_for_imported_list_and_add_parts_to_model() + + # check indexable used when needed in key + aps = rqp.AttributePropertySet(support = grid, indexable = 'cells') + assert aps is not None + assert len(aps) == 13 + assert 'facies' in aps.keys() + assert 'facies_columns' in aps.keys() + + # check duplicate ignored + aps = rqp.AttributePropertySet(support = grid, indexable = None, multiple_handling = 'ignore') + assert aps is not None + assert len(aps) == 13 + key_list = list(aps.keys()) + assert len(key_list) == 13 # all parts still exist in the PropertyCollection + assert len( + set(key_list)) == 12 # but two will have the same aps key (only the first is visible as AttributeProperty) + assert 'facies' in aps.keys() + assert 'facies_columns' not in aps.keys() + + # check that multiple handling 'exception' raises key error + with pytest.raises(KeyError) as e_info: + aps = rqp.AttributePropertySet(support = grid, multiple_handling = 'exception') + + # check that multiple handling 'warn' generates log warning + aps = rqp.AttributePropertySet(support = grid, multiple_handling = 'warn') + assert len(caplog.records) > 0 + assert caplog.records[-1].getMessage().endswith( + "duplicate key in AttributePropertySet; only first instance included: facies") + assert aps is not None + assert len(aps) == 13 + key_list = list(aps.keys()) + assert len(key_list) == 13 + assert len(set(key_list)) == 12 + assert 'facies' in aps.keys() + assert 'facies_columns' not in aps.keys() + + def test_load_attribute_property_collection_title(example_model_with_prop_ts_rels): # Arrange model = example_model_with_prop_ts_rels