From 825e31f934790d0bee59a6dd33f625691941a963 Mon Sep 17 00:00:00 2001 From: Matthew Avaylon Date: Mon, 29 Jan 2024 14:46:14 -0800 Subject: [PATCH 1/5] Fix the path for finding the herd zip file. (#1046) * fix * test * test * Update CHANGELOG.md --- CHANGELOG.md | 1 + src/hdmf/common/resources.py | 11 ++++++++++- tests/unit/common/test_resources.py | 20 ++++++++++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ccb6bc516..5894f38ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## HDMF 3.12.1 (Upcoming) ### 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) diff --git a/src/hdmf/common/resources.py b/src/hdmf/common/resources.py index 29d61ea79..fdca4bb81 100644 --- a/src/hdmf/common/resources.py +++ b/src/hdmf/common/resources.py @@ -989,6 +989,15 @@ 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): @@ -996,7 +1005,7 @@ 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) diff --git a/tests/unit/common/test_resources.py b/tests/unit/common/test_resources.py index 8cbd8291e..0de6ba644 100644 --- a/tests/unit/common/test_resources.py +++ b/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']) From 3fbe27d597a00f00740fb536860465634e3b1ce9 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 4 Feb 2024 17:22:22 -0800 Subject: [PATCH 2/5] Bump codecov/codecov-action from 3 to 4 (#1052) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Ryan Ly --- .github/workflows/run_coverage.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/run_coverage.yml b/.github/workflows/run_coverage.yml index 4e43afc7b..7a18e5068 100644 --- a/.github/workflows/run_coverage.yml +++ b/.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 }} From 46d81cb23429076e452484c155433882d0df6575 Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Mon, 5 Feb 2024 01:17:19 -0800 Subject: [PATCH 3/5] Fix validator not validating on spec of builder data type (#1050) --- CHANGELOG.md | 1 + src/hdmf/validate/validator.py | 21 +++++++-- tests/unit/validator_tests/test_validate.py | 52 +++++++++++++++++++++ 3 files changed, 70 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5894f38ae..8e7888a54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - 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) ## HDMF 3.12.0 (January 16, 2024) diff --git a/src/hdmf/validate/validator.py b/src/hdmf/validate/validator.py index 6bea85975..bdfc15f8f 100644 --- a/src/hdmf/validate/validator.py +++ b/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) diff --git a/tests/unit/validator_tests/test_validate.py b/tests/unit/validator_tests/test_validate.py index 7002ebd6f..95ff5d98e 100644 --- a/tests/unit/validator_tests/test_validate.py +++ b/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') From 232e74eb7134685c2e95cd06011aa9b162f9cb25 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 5 Feb 2024 13:34:30 -0800 Subject: [PATCH 4/5] [pre-commit.ci] pre-commit autoupdate (#1054) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2bd0609bb..ad798b8f7 100644 --- a/.pre-commit-config.yaml +++ b/.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 From 8ad4db2e779d1729a000a5665dd35859b45ca47a Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Tue, 6 Feb 2024 14:32:43 -0800 Subject: [PATCH 5/5] Prepare for release of HDMF 3.12.1 (#1053) --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e7888a54..e309b47fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # 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)