Do not set data_type_inc=NWBContainer for NWBContainer (#2135)

Co-authored-by: Steph Prince <[email protected]>

Author: rly

CHANGELOG.md

@@ -4,7 +4,9 @@
 
 ### Fixed
 - Fixed incorrect warning for path not ending in `.nwb` when no path argument was provided. @t-b [#2130](https://github.com/NeurodataWithoutBorders/pynwb/pull/2130)
+- Fixed issue with setting `neurodata_type_inc` when reading NWB files with cached schema versions less than 2.2.0. @rly [#2135](https://github.com/NeurodataWithoutBorders/pynwb/pull/2135)
 - Fixed import structure test. @rly [#2136](https://github.com/NeurodataWithoutBorders/pynwb/pull/2136)
+
 ### Documentation and tutorial enhancements
 - Change UI of assistant to be an accordion that is always visible. [#2124](https://github.com/NeurodataWithoutBorders/pynwb/pull/2124)
 

src/pynwb/spec.py

@@ -150,7 +150,8 @@ class NWBDatasetSpec(BaseStorageOverride, DatasetSpec):
     def __init__(self, **kwargs):
         kwargs = self._translate_kwargs(kwargs)
         # set data_type_inc to NWBData only if it is not specified and the type is not an HDMF base type
-        if kwargs['data_type_inc'] is None and kwargs['data_type_def'] not in (None, 'Data'):
+        exclude_set_data_type_inc_types = (None, 'Data', 'NWBData')
+        if kwargs['data_type_inc'] is None and kwargs['data_type_def'] not in exclude_set_data_type_inc_types:
             kwargs['data_type_inc'] = 'NWBData'
         super().__init__(**kwargs)
 
@@ -167,10 +168,13 @@ class NWBGroupSpec(BaseStorageOverride, GroupSpec):
     @docval(*deepcopy(_group_docval))
     def __init__(self, **kwargs):
         kwargs = self._translate_kwargs(kwargs)
-        # set data_type_inc to NWBData only if it is not specified and the type is not an HDMF base type
-        # NOTE: CSRMatrix in hdmf-common-schema does not have a data_type_inc but should not inherit from
-        # NWBContainer. This will be fixed in hdmf-common-schema 1.2.1.
-        if kwargs['data_type_inc'] is None and kwargs['data_type_def'] not in (None, 'Container', 'CSRMatrix'):
+        # set data_type_inc to NWBContainer only if it is not specified or special cases
+        # NOTE: NWBContainer in nwb-schema < 2.2.0 had no neurodata_type_inc, so we need to exclude it here to avoid
+        # setting data_type_inc to NWBContainer for NWBContainer itself
+        # NOTE: CSRMatrix in hdmf-common-schema < 1.2.1 had no data_type_inc, but should not inherit from
+        # NWBContainer.
+        exclude_set_data_type_inc_types = (None, 'Container', 'NWBContainer', 'CSRMatrix')
+        if kwargs['data_type_inc'] is None and kwargs['data_type_def'] not in exclude_set_data_type_inc_types:
             kwargs['data_type_inc'] = 'NWBContainer'
         super().__init__(**kwargs)
 

tests/unit/test_spec.py

@@ -7,11 +7,11 @@
 import json
 
 from pynwb.spec import (
-    NWBNamespaceBuilder, 
-    NWBRefSpec, 
-    NWBLinkSpec, 
+    NWBNamespaceBuilder,
+    NWBRefSpec,
+    NWBLinkSpec,
     NWBDtypeSpec,
-    NWBGroupSpec, 
+    NWBGroupSpec,
     NWBDatasetSpec
 )
 from pynwb.testing import TestCase
@@ -116,3 +116,74 @@ def test_add_dataset(self):
         self.assertEqual(len(spec.datasets), 1)
         self.assertEqual(spec.datasets[0].name, 'dataset1')
         self.assertIsInstance(spec.datasets[0], NWBDatasetSpec)
+
+    def test_set_neurodata_type_inc(self):
+        # neurodata_type_inc should be set to NWBContainer if not specified and neurodata_type_def is not a base type
+        spec = NWBGroupSpec(
+            doc='A test group',
+            neurodata_type_def='MyCustomType',
+            name='Group1',
+        )
+        self.assertEqual(spec.neurodata_type_inc, 'NWBContainer')
+
+        # neurodata_type_inc should not be set to NWBContainer if neurodata_type_def is a base type
+        spec = NWBGroupSpec(
+            doc='A test group',
+            neurodata_type_def='Container',
+            name='Group1',
+        )
+        self.assertIsNone(spec.neurodata_type_inc)
+
+        # neurodata_type_inc should not be set to NWBContainer if neurodata_type_def is a base type
+        spec = NWBGroupSpec(
+            doc='A test group',
+            neurodata_type_def='NWBContainer',
+            name='Group1',
+        )
+        self.assertIsNone(spec.neurodata_type_inc)
+
+        # neurodata_type_inc should not be set to NWBContainer if it is explicitly specified
+        spec = NWBGroupSpec(
+            doc='A test group',
+            neurodata_type_def='MyCustomType',
+            neurodata_type_inc='SomeOtherType',
+            name='Group1',
+        )
+        self.assertEqual(spec.neurodata_type_inc, 'SomeOtherType')
+
+
+class NWBDatasetSpecTest(TestCase):
+
+    def test_set_neurodata_type_inc(self):
+        # neurodata_type_inc should be set to NWBData if not specified and neurodata_type_def is not a base type
+        spec = NWBDatasetSpec(
+            doc='A test dataset',
+            neurodata_type_def='MyCustomType',
+            name='dataset1',
+        )
+        self.assertEqual(spec.neurodata_type_inc, 'NWBData')
+
+        # neurodata_type_inc should not be set to NWBData if neurodata_type_def is a base type
+        spec = NWBDatasetSpec(
+            doc='A test dataset',
+            neurodata_type_def='Data',
+            name='dataset1',
+        )
+        self.assertIsNone(spec.neurodata_type_inc)
+
+        # neurodata_type_inc should not be set to NWBDataset if neurodata_type_def is a base type
+        spec = NWBDatasetSpec(
+            doc='A test dataset',
+            neurodata_type_def='NWBData',
+            name='dataset1',
+        )
+        self.assertIsNone(spec.neurodata_type_inc)
+
+        # neurodata_type_inc should not be set to NWBData if it is explicitly specified
+        spec = NWBDatasetSpec(
+            doc='A test dataset',
+            neurodata_type_def='MyCustomType',
+            neurodata_type_inc='SomeOtherType',
+            name='dataset1',
+        )
+        self.assertEqual(spec.neurodata_type_inc, 'SomeOtherType')