From 6d0be176d1a16ace594410404753aa9d69d4a2ed Mon Sep 17 00:00:00 2001 From: Matthew Avaylon Date: Wed, 10 Apr 2024 09:27:54 -0700 Subject: [PATCH] Field_config refactored (#1094) * Field_config needs to be able to use different type maps * refactor * refactor * coverage * cov * Update CHANGELOG.md * coverage * cov --- CHANGELOG.md | 3 +- src/hdmf/container.py | 74 +++++++++++++++++--------------- tests/unit/common/test_common.py | 7 ++- tests/unit/hdmf_config.yaml | 5 +++ tests/unit/hdmf_config2.yaml | 2 +- tests/unit/test_term_set.py | 49 ++++++++++++++------- 6 files changed, 85 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e72015601..00eeeb5dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,8 @@ ## HDMF 3.14.0 (Upcoming) ### Enhancements -- Added `TermSetConfigurator` to automatically wrap fields with `TermSetWrapper` according to a configuration file. @mavaylon1 [#1016](https://github.com/hdmf-dev/hdmf/pull/1016) +- Updated `_field_config` to take `type_map` as an argument for APIs. @mavaylon1 [#1094](https://github.com/hdmf-dev/hdmf/pull/1094) +- Added `TypeConfigurator` to automatically wrap fields with `TermSetWrapper` according to a configuration file. @mavaylon1 [#1016](https://github.com/hdmf-dev/hdmf/pull/1016) - Updated `TermSetWrapper` to support validating a single field within a compound array. @mavaylon1 [#1061](https://github.com/hdmf-dev/hdmf/pull/1061) ## HDMF 3.13.0 (March 20, 2024) diff --git a/src/hdmf/container.py b/src/hdmf/container.py index f93c06199..ca2c5252b 100644 --- a/src/hdmf/container.py +++ b/src/hdmf/container.py @@ -86,10 +86,15 @@ def setter(self, val): if name in self.fields: msg = "can't set attribute '%s' -- already set" % name raise AttributeError(msg) - self.fields[name] = self._field_config(arg_name=name, val=val) - + self.fields[name] = self._field_config(arg_name=name, + val=val, + type_map=self._get_type_map()) return setter + def _get_type_map(self): + from hdmf.common import get_type_map # circular import + return get_type_map() + @property def data_type(self): """ @@ -97,8 +102,7 @@ def data_type(self): """ return getattr(self, self._data_type_attr) - - def _field_config(self, arg_name, val): + def _field_config(self, arg_name, val, type_map): """ This method will be called in the setter. The termset configuration will be used (if loaded) to check for a defined TermSet associated with the field. If found, the value of the field @@ -108,9 +112,12 @@ def _field_config(self, arg_name, val): itself is only one file. When a user loads custom configs, the config is appended/modified. The modifications are not written to file, avoiding permanent modifications. """ - # load termset configuration file from global Config - from hdmf.common import get_type_map # circular import - type_map = get_type_map() + # If the val has been manually wrapped then skip checking the config for the attr + if isinstance(val, TermSetWrapper): + msg = "Field value already wrapped with TermSetWrapper." + warn(msg) + return val + configurator = type_map.type_config if len(configurator.path)>0: @@ -119,8 +126,9 @@ def _field_config(self, arg_name, val): termset_config = configurator.config else: return val + # check to see that the namespace for the container is in the config - if self.namespace not in type_map.container_types: + if self.namespace not in termset_config['namespaces']: msg = "%s not found within loaded configuration." % self.namespace warn(msg) return val @@ -134,33 +142,29 @@ def _field_config(self, arg_name, val): warn(msg) return val else: - for attr in config_namespace['data_types'][data_type]: - obj_mapper = type_map.get_map(self) - - # get the spec according to attr name in schema - # Note: this is the name for the field in the config - spec = obj_mapper.get_attr_spec(attr) - - # In the case of dealing with datasets directly or not defined in the spec. - # (Data/VectorData/DynamicTable/etc) - if spec is None: - msg = "Spec not found for %s." % attr - warn(msg) - return val - else: - # If the val has been manually wrapped then skip checking the config for the attr - if isinstance(val, TermSetWrapper): - msg = "Field value already wrapped with TermSetWrapper." - warn(msg) - return val - else: - # From the spec, get the mapped attribute name - mapped_attr_name = obj_mapper.get_attribute(spec) - termset_path = os.path.join(CUR_DIR, - config_namespace['data_types'][data_type][mapped_attr_name]['termset']) - termset = TermSet(term_schema_path=termset_path) - val = TermSetWrapper(value=val, termset=termset) - return val + # Get the ObjectMapper + obj_mapper = type_map.get_map(self) + + # Get the spec for the constructor arg + spec = obj_mapper.get_carg_spec(arg_name) + if spec is None: + msg = "Spec not found for %s." % arg_name + warn(msg) + return val + + # Get spec attr name + mapped_attr_name = obj_mapper.get_attribute(spec) + + config_data_type = config_namespace['data_types'][data_type] + try: + config_termset_path = config_data_type[mapped_attr_name] + except KeyError: + return val + + termset_path = os.path.join(CUR_DIR, config_termset_path['termset']) + termset = TermSet(term_schema_path=termset_path) + val = TermSetWrapper(value=val, termset=termset) + return val @classmethod def _getter(cls, field): diff --git a/tests/unit/common/test_common.py b/tests/unit/common/test_common.py index e20614852..730c60404 100644 --- a/tests/unit/common/test_common.py +++ b/tests/unit/common/test_common.py @@ -17,8 +17,11 @@ def test_copy_ts_config(self): load_type_config(config_path=path) tm = get_type_map() config = {'namespaces': {'hdmf-common': {'version': '3.12.2', - 'data_types': {'VectorData': {'description': {'termset': 'example_test_term_set.yaml'}}, - 'VectorIndex': {'data': '...'}}}}} + 'data_types': {'VectorData': + {'description': {'termset': 'example_test_term_set.yaml'}}, + 'VectorIndex': {'data': '...'}}}, 'foo_namespace': + {'version': '...', 'data_types': + {'ExtensionContainer': {'description': None}}}}} self.assertEqual(tm.type_config.config, config) self.assertEqual(tm.type_config.path, [path]) diff --git a/tests/unit/hdmf_config.yaml b/tests/unit/hdmf_config.yaml index 92ec2f321..5156c5828 100644 --- a/tests/unit/hdmf_config.yaml +++ b/tests/unit/hdmf_config.yaml @@ -7,3 +7,8 @@ namespaces: termset: example_test_term_set.yaml VectorIndex: data: ... + foo_namespace: + version: ... + data_types: + ExtensionContainer: + description: diff --git a/tests/unit/hdmf_config2.yaml b/tests/unit/hdmf_config2.yaml index 0aecacf51..f51113d53 100644 --- a/tests/unit/hdmf_config2.yaml +++ b/tests/unit/hdmf_config2.yaml @@ -9,7 +9,7 @@ namespaces: description: termset: example_test_term_set.yaml VectorData: - description: ... + name: namespace2: version: 0 data_types: diff --git a/tests/unit/test_term_set.py b/tests/unit/test_term_set.py index 1d7721f1b..6c944999b 100644 --- a/tests/unit/test_term_set.py +++ b/tests/unit/test_term_set.py @@ -4,7 +4,7 @@ from hdmf import Container from hdmf.term_set import TermSet, TermSetWrapper, TypeConfigurator from hdmf.testing import TestCase, remove_test_file -from hdmf.common import (VectorIndex, VectorData, unload_type_config, +from hdmf.common import (VectorData, unload_type_config, get_loaded_type_config, load_type_config) from hdmf.utils import popargs @@ -267,12 +267,15 @@ def test_load_two_unique_configs(self): tc = TypeConfigurator(path=path) tc.load_type_config(config_path=path2) config = {'namespaces': {'hdmf-common': {'version': '3.12.2', - 'data_types': {'VectorData': {'description': '...'}, - 'VectorIndex': {'data': '...'}, - 'Data': {'description': {'termset': 'example_test_term_set.yaml'}}, - 'EnumData': {'description': {'termset': 'example_test_term_set.yaml'}}}}, - 'namespace2': {'version': 0, - 'data_types': {'MythicData': {'description': {'termset': 'example_test_term_set.yaml'}}}}}} + 'data_types': {'VectorData': {'name': None}, + 'VectorIndex': {'data': '...'}, + 'Data': {'description': {'termset': 'example_test_term_set.yaml'}}, + 'EnumData': {'description': {'termset': 'example_test_term_set.yaml'}}}}, + 'foo_namespace': {'version': '...', + 'data_types': {'ExtensionContainer': {'description': None}}}, + 'namespace2': {'version': 0, 'data_types': + {'MythicData': {'description': + {'termset': 'example_test_term_set.yaml'}}}}}} self.assertEqual(tc.path, [path, path2]) self.assertEqual(tc.config, config) @@ -286,6 +289,13 @@ def __init__(self, **kwargs): super().__init__(**kwargs) self.description = description + @property + def data_type(self): + """ + Return the spec data type associated with this container. + """ + return "ExtensionContainer" + class TestGlobalTypeConfig(TestCase): def setUp(self): @@ -300,8 +310,12 @@ def test_load_config(self): config = get_loaded_type_config() self.assertEqual(config, {'namespaces': {'hdmf-common': {'version': '3.12.2', - 'data_types': {'VectorData': {'description': {'termset': 'example_test_term_set.yaml'}}, - 'VectorIndex': {'data': '...'}}}}}) + 'data_types': {'VectorData': + {'description': {'termset': 'example_test_term_set.yaml'}}, + 'VectorIndex': {'data': '...'}}}, 'foo_namespace': + {'version': '...', 'data_types': + {'ExtensionContainer': {'description': None}}}}} +) def test_validate_with_config(self): data = VectorData(name='foo', data=[0], description='Homo sapiens') @@ -326,11 +340,14 @@ def test_already_wrapped_warn(self): data=[0], description=TermSetWrapper(value='Homo sapiens', termset=terms)) - def test_warn_field_not_in_spec(self): - col1 = VectorData(name='col1', - description='Homo sapiens', - data=['1a', '1b', '1c', '2a']) + def test_field_not_in_config(self): + unload_type_config() + load_type_config(config_path='tests/unit/hdmf_config2.yaml') + + VectorData(name='foo', data=[0], description='Homo sapiens') + + def test_spec_none(self): with self.assertWarns(Warning): - VectorIndex(name='col1_index', - target=col1, - data=[3, 4]) + ExtensionContainer(name='foo', + namespace='foo_namespace', + description='Homo sapiens')