Merge branch 'dev' into jupyter-recursion-error

.github/workflows/run_coverage.yml

@@ -65,6 +65,8 @@ jobs:
           python -m coverage report -m
 
       - name: Upload coverage to Codecov
-        uses: codecov/codecov-action@v3
+        uses: codecov/codecov-action@v4
         with:
           fail_ci_if_error: true
+        env:
+          CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}

.pre-commit-config.yaml

@@ -18,7 +18,7 @@ repos:
 #     hooks:
 #     -   id: black
 -   repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.1.14
+    rev: v0.2.0
     hooks:
     -   id: ruff
 # -   repo: https://github.com/econchick/interrogate

CHANGELOG.md

@@ -1,10 +1,12 @@
 # HDMF Changelog
 
-## HDMF 3.12.1 (Upcoming)
+## HDMF 3.12.1 (February 5, 2024)
 
 ### Bug fixes
+- Fixed retrieving the correct path for a `HERD` zip file on read. [#1046](https://github.com/hdmf-dev/hdmf/pull/1046)
 - Fixed internal links in docstrings and tutorials. @stephprince [#1031](https://github.com/hdmf-dev/hdmf/pull/1031)
 - Fixed issue with creating documentation links to classes in docval arguments. @rly [#1036](https://github.com/hdmf-dev/hdmf/pull/1036)
+- Fixed issue with validator not validating against the spec that defines the data type of the builder. @rly [#1050](https://github.com/hdmf-dev/hdmf/pull/1050)
 - Fixed recursion error in html representation generation in jupyter notebooks. @stephprince [#1038](https://github.com/hdmf-dev/hdmf/pull/1038)
 
 ## HDMF 3.12.0 (January 16, 2024)

src/hdmf/common/resources.py

@@ -989,14 +989,23 @@ def to_zip(self, **kwargs):
         for file in files:
             os.remove(file)
 
+    @classmethod
+    @docval({'name': 'path', 'type': str, 'doc': 'The path to the zip file.'})
+    def get_zip_directory(cls, path):
+        """
+        Return the directory of the file given.
+        """
+        directory = os.path.dirname(os.path.realpath(path))
+        return directory
+
     @classmethod
     @docval({'name': 'path', 'type': str, 'doc': 'The path to the zip file.'})
     def from_zip(cls, **kwargs):
         """
         Method to read in zipped tsv files to populate HERD.
         """
         zip_file = kwargs['path']
-        directory = os.path.dirname(zip_file)
+        directory = cls.get_zip_directory(zip_file)
 
         with zipfile.ZipFile(zip_file, 'r') as zip:
             zip.extractall(directory)

src/hdmf/validate/validator.py

@@ -545,7 +545,8 @@ def __validate_child_builder(self, child_spec, child_builder, parent_builder):
                 yield self.__construct_illegal_link_error(child_spec, parent_builder)
                 return  # do not validate illegally linked objects
             child_builder = child_builder.builder
-        for child_validator in self.__get_child_validators(child_spec):
+        child_builder_data_type = child_builder.attributes.get(self.spec.type_key())
+        for child_validator in self.__get_child_validators(child_spec, child_builder_data_type):
             yield from child_validator.validate(child_builder)
 
     def __construct_illegal_link_error(self, child_spec, parent_builder):
@@ -557,7 +558,7 @@ def __construct_illegal_link_error(self, child_spec, parent_builder):
     def __cannot_be_link(spec):
         return not isinstance(spec, LinkSpec) and not spec.linkable
 
-    def __get_child_validators(self, spec):
+    def __get_child_validators(self, spec, builder_data_type):
         """Returns the appropriate list of validators for a child spec
 
         Due to the fact that child specs can both inherit a data type via data_type_inc
@@ -572,9 +573,21 @@ def __get_child_validators(self, spec):
         returned. If the spec is a LinkSpec, no additional Validator is returned
         because the LinkSpec cannot add or modify fields and the target_type will be
         validated by the Validator returned from the ValidatorMap.
+
+        For example, if the spec is:
+        {'doc': 'Acquired, raw data.', 'quantity': '*', 'data_type_inc': 'NWBDataInterface'}
+        then the returned validators will be:
+        - a validator for the spec for the builder data type
+        - a validator for the spec for data_type_def: NWBDataInterface
+        - a validator for the above spec which might have extended properties
+          on top of data_type_def: NWBDataInterface
         """
-        if _resolve_data_type(spec) is not None:
-            yield self.vmap.get_validator(_resolve_data_type(spec))
+        if builder_data_type is not None:
+            yield self.vmap.get_validator(builder_data_type)
+
+        spec_data_type = _resolve_data_type(spec)
+        if spec_data_type is not None:
+            yield self.vmap.get_validator(spec_data_type)
 
         if isinstance(spec, GroupSpec):
             yield GroupValidator(spec, self.vmap)

tests/unit/common/test_resources.py

@@ -52,6 +52,7 @@ def remove_er_files(self):
         remove_test_file('./keys.tsv')
         remove_test_file('./files.tsv')
         remove_test_file('./HERD.zip')
+        remove_test_file('./HERD2.zip')
 
     def child_tsv(self, external_resources):
         for child in external_resources.children:
@@ -737,6 +738,25 @@ def test_to_and_from_zip(self):
 
         self.remove_er_files()
 
+    def test_get_zip_directory(self):
+        er = HERD()
+        data = Data(name="species", data=['Homo sapiens', 'Mus musculus'])
+        er.add_ref(file=HERDManagerContainer(name='file'),
+                   container=data,
+                   key='key1',
+                   entity_id='entity_id1',
+                   entity_uri='entity1')
+
+        er.to_zip(path='./HERD.zip')
+        er.to_zip(path='HERD2.zip')
+
+        d1 = er.get_zip_directory('./HERD.zip')
+        d2 = er.get_zip_directory('HERD2.zip')
+
+        self.assertEqual(d1,d2)
+
+        self.remove_er_files()
+
     def test_to_and_from_zip_entity_value_error(self):
         er = HERD()
         data = Data(name="species", data=['Homo sapiens', 'Mus musculus'])

tests/unit/validator_tests/test_validate.py

@@ -1217,3 +1217,55 @@ def test_empty_int_dataset(self):
                                          DatasetBuilder(name='dataInt', data=[], dtype='int')  # <-- Empty int dataset
                                          ])
         self.runBuilderRoundTrip(builder)
+
+
+class TestValidateSubspec(ValidatorTestBase):
+    """When a subtype satisfies a subspec, the validator should also validate
+    that the subtype satisfies its spec.
+    """
+
+    def getSpecs(self):
+        dataset_spec = DatasetSpec('A dataset', data_type_def='Foo')
+        sub_dataset_spec = DatasetSpec(
+            doc='A subtype of Foo',
+            data_type_def='Bar',
+            data_type_inc='Foo',
+            attributes=[
+                AttributeSpec(name='attr1', doc='an example attribute', dtype='text')
+            ],
+        )
+        spec = GroupSpec(
+            doc='A group that contains a Foo',
+            data_type_def='Baz',
+            datasets=[
+                DatasetSpec(doc='Child Dataset', data_type_inc='Foo'),
+            ])
+        return (spec, dataset_spec, sub_dataset_spec)
+
+    def test_validate_subtype(self):
+        """Test that when spec A contains dataset B, and C is a subtype of B, using a C builder is valid.
+        """
+        builder = GroupBuilder(
+            name='my_baz',
+            attributes={'data_type': 'Baz'},
+            datasets=[
+                DatasetBuilder(name='bar', attributes={'data_type': 'Bar', 'attr1': 'value'})
+            ],
+        )
+        result = self.vmap.validate(builder)
+        self.assertEqual(len(result), 0)
+
+    def test_validate_subtype_error(self):
+        """Test that when spec A contains dataset B, and C is a subtype of B, using a C builder validates
+        against spec C.
+        """
+        builder = GroupBuilder(
+            name='my_baz',
+            attributes={'data_type': 'Baz'},
+            datasets=[
+                DatasetBuilder(name='bar', attributes={'data_type': 'Bar'})
+            ],
+        )
+        result = self.vmap.validate(builder)
+        self.assertEqual(len(result), 1)
+        self.assertValidationError(result[0], MissingError, name='Bar/attr1')