Field_config refactored (#1094)

* Field_config needs to be able to use different type maps * refactor * refactor * coverage * cov * Update CHANGELOG.md * coverage * cov
hdmf-dev · Apr 10, 2024 · 6d0be17 · 6d0be17
1 parent d85d0cb
commit 6d0be17
Show file tree

Hide file tree

Showing 6 changed files with 85 additions and 55 deletions.
diff --git 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
@@ -86,19 +86,23 @@ 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):
         """
         Return the spec data type associated with this container.
         """
         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
@@ -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
@@ -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
@@ -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
@@ -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')