diff --git a/hed/schema/hed_schema_section.py b/hed/schema/hed_schema_section.py index 9798e1ef5..c0cf21cfc 100644 --- a/hed/schema/hed_schema_section.py +++ b/hed/schema/hed_schema_section.py @@ -212,17 +212,20 @@ def _check_if_duplicate(self, name, new_entry): return new_entry def _add_to_dict(self, name, new_entry, parent_index=None): + if new_entry.has_attribute(HedKey.Rooted): + del new_entry.attributes[HedKey.Rooted] if new_entry.has_attribute(HedKey.InLibrary): parent_name = new_entry.parent_name if parent_name.lower() in self: # Make sure we insert the new entry after all previous relevant ones, as order isn't assured # for rooted tags parent_entry = self.get(parent_name.lower()) - parent_index = self.all_entries.index(parent_entry) + 1 + parent_index = self.all_entries.index(parent_entry) for i in range(parent_index, len(self.all_entries)): - parent_index = i + 1 - if not self.all_entries[i].name.startswith(parent_entry.name): - break + if self.all_entries[i].name.startswith(parent_entry.name): + parent_index = i + 1 + continue + break return super()._add_to_dict(name, new_entry, parent_index) diff --git a/hed/schema/schema_compliance.py b/hed/schema/schema_compliance.py index 84ba8e85a..ddb222663 100644 --- a/hed/schema/schema_compliance.py +++ b/hed/schema/schema_compliance.py @@ -49,7 +49,7 @@ def check_compliance(hed_schema, check_for_warnings=True, name=None, error_handl HedKey.RelatedTag: tag_exists_check, HedKey.UnitClass: tag_is_placeholder_check, HedKey.ValueClass: tag_is_placeholder_check, - HedKey.Rooted: attribute_does_not_exist_check, # This should be impossible to trigger unless loading fails + HedKey.Rooted: tag_exists_base_schema_check, } # Check attributes @@ -170,6 +170,33 @@ def tag_exists_check(hed_schema, tag_entry, possible_tags, force_issues_as_warni return issues +def tag_exists_base_schema_check(hed_schema, tag_entry, tag_name, force_issues_as_warnings=True): + """ Check if the single tag is a partnered schema tag + + Parameters: + hed_schema (HedSchema): The schema to check if the tag exists. + tag_entry (HedSchemaEntry): The schema entry for this tag. + tag_name (str): The tag to verify, can be any form. + force_issues_as_warnings (bool): If True, set all the severity levels to warning. + + Returns: + list: A list of issues. Each issue is a dictionary. + + """ + issues = [] + rooted_tag = tag_name.lower() + if rooted_tag not in hed_schema.all_tags: + issues += ErrorHandler.format_error(ValidationErrors.NO_VALID_TAG_FOUND, + rooted_tag, + index_in_tag=0, + index_in_tag_end=len(rooted_tag)) + + if force_issues_as_warnings: + for issue in issues: + issue['severity'] = ErrorSeverity.WARNING + return issues + + def validate_schema_term(hed_term): """ Check short tag for capitalization and illegal characters. diff --git a/hed/schema/schema_io/schema2base.py b/hed/schema/schema_io/schema2base.py index bb5240134..d7c717476 100644 --- a/hed/schema/schema_io/schema2base.py +++ b/hed/schema/schema_io/schema2base.py @@ -69,15 +69,6 @@ def _write_tag_entry(self, tag_entry, parent=None, level=0): def _write_entry(self, entry, parent_node, include_props=True): raise NotImplementedError("This needs to be defined in the subclass") - def _write_rooted_parent_entry(self, tag_entry, schema_node): - parent_copy = copy.deepcopy(tag_entry._parent_tag) - parent_copy.attributes = {HedKey.Rooted: True} - parent_copy.description = "" - - parent_node = self._write_tag_entry(parent_copy, schema_node, 0) - - return parent_node - def _output_tags(self, all_tags): schema_node = self._start_section(HedSectionKey.AllTags) @@ -98,11 +89,11 @@ def _output_tags(self, all_tags): else: # Only output the rooted parent nodes if they have a parent(for duplicates that don't) if tag_entry.has_attribute(HedKey.InLibrary) and tag_entry.parent and \ - not tag_entry.parent.has_attribute(HedKey.InLibrary): + not tag_entry.parent.has_attribute(HedKey.InLibrary) and not self._save_merged: if tag_entry.parent.name not in all_nodes: - parent_node = self._write_rooted_parent_entry(tag_entry, schema_node) - all_nodes[tag_entry.parent.name] = parent_node - level_adj = level - 1 + level_adj = level + tag_entry = copy.deepcopy(tag_entry) + tag_entry.attributes[HedKey.Rooted] = tag_entry.parent.short_tag_name parent_node = all_nodes.get(tag_entry.parent_name, schema_node) child_node = self._write_tag_entry(tag_entry, parent_node, level - level_adj) diff --git a/hed/schema/schema_io/wiki2schema.py b/hed/schema/schema_io/wiki2schema.py index 16ce95390..c09a6803b 100644 --- a/hed/schema/schema_io/wiki2schema.py +++ b/hed/schema/schema_io/wiki2schema.py @@ -305,11 +305,12 @@ def _read_schema(self, lines): continue try: - rooted_entry = schema_validation_util.check_rooted_errors(tag_entry, self._schema, self._loading_merged) + rooted_entry = schema_validation_util.find_rooted_entry(tag_entry, self._schema, self._loading_merged) if rooted_entry: parent_tags = rooted_entry.long_tag_name.split("/") - level_adj = len(parent_tags) - 1 - continue + level_adj = len(parent_tags) + # Create the entry again for rooted tags, to get the full name. + tag_entry = self._add_tag_line(parent_tags, line_number, line) except HedFileError as e: self._add_fatal_error(line_number, line, e.message, e.code) continue diff --git a/hed/schema/schema_io/xml2schema.py b/hed/schema/schema_io/xml2schema.py index ae0a8f246..b6311debe 100644 --- a/hed/schema/schema_io/xml2schema.py +++ b/hed/schema/schema_io/xml2schema.py @@ -122,11 +122,14 @@ def _populate_tag_dictionaries(self): tag = tag.replace(loading_from_chain_short, loading_from_chain) tag_entry = self._parse_node(tag_element, HedSectionKey.AllTags, tag) - rooted_entry = schema_validation_util.check_rooted_errors(tag_entry, self._schema, self._loading_merged) + rooted_entry = schema_validation_util.find_rooted_entry(tag_entry, self._schema, self._loading_merged) if rooted_entry: - loading_from_chain = rooted_entry.name - loading_from_chain_short = rooted_entry.short_tag_name - continue + loading_from_chain = rooted_entry.name + "/" + tag_entry.short_tag_name + loading_from_chain_short = tag_entry.short_tag_name + + tag = tag.replace(loading_from_chain_short, loading_from_chain) + tag_entry = self._parse_node(tag_element, HedSectionKey.AllTags, tag) + self._add_to_dict(tag_entry, HedSectionKey.AllTags) def _populate_unit_class_dictionaries(self): diff --git a/hed/schema/schema_validation_util.py b/hed/schema/schema_validation_util.py index 64cac9980..e6de68e3d 100644 --- a/hed/schema/schema_validation_util.py +++ b/hed/schema/schema_validation_util.py @@ -99,7 +99,7 @@ def validate_attributes(attrib_dict, filename): # Might move this to a baseclass version if one is ever made for wiki2schema/xml2schema -def check_rooted_errors(tag_entry, schema, loading_merged): +def find_rooted_entry(tag_entry, schema, loading_merged): """ This semi-validates rooted tags, raising an exception on major errors Parameters: @@ -115,11 +115,8 @@ def check_rooted_errors(tag_entry, schema, loading_merged): HedValueError: Raises if the tag doesn't exist or similar """ - if tag_entry.has_attribute(constants.HedKey.Rooted): - if tag_entry.parent_name: - raise HedFileError(HedExceptions.ROOTED_TAG_INVALID, - f'Found rooted tag \'{tag_entry.short_tag_name}\' as a non root node.', - schema.filename) + rooted_tag = tag_entry.has_attribute(constants.HedKey.Rooted, return_value=True) + if rooted_tag is not None: if not schema.with_standard: raise HedFileError(HedExceptions.ROOTED_TAG_INVALID, f"Rooted tag attribute found on '{tag_entry.short_tag_name}' in a standard schema.", @@ -129,7 +126,17 @@ def check_rooted_errors(tag_entry, schema, loading_merged): f'Found rooted tag \'{tag_entry.short_tag_name}\' in schema without unmerged="True"', schema.filename) - rooted_entry = schema.all_tags.get(tag_entry.name.lower()) + if not isinstance(rooted_tag, str): + raise HedFileError(HedExceptions.ROOTED_TAG_INVALID, + f'Rooted tag \'{tag_entry.short_tag_name}\' is not a string."', + schema.filename) + + if tag_entry.parent_name: + raise HedFileError(HedExceptions.ROOTED_TAG_INVALID, + f'Found rooted tag \'{tag_entry.short_tag_name}\' as a non root node.', + schema.filename) + + rooted_entry = schema.all_tags.get(rooted_tag) if not rooted_entry or rooted_entry.has_attribute(constants.HedKey.InLibrary): raise HedFileError(HedExceptions.ROOTED_TAG_DOES_NOT_EXIST, f"Rooted tag '{tag_entry.short_tag_name}' not found in paired standard schema", diff --git a/spec_tests/hed-specification b/spec_tests/hed-specification index 4909076a4..d7d174fde 160000 --- a/spec_tests/hed-specification +++ b/spec_tests/hed-specification @@ -1 +1 @@ -Subproject commit 4909076a416ee9645c8c539d77cdb9750172d154 +Subproject commit d7d174fde455b42c0c0a0534b3566f76fac53496 diff --git a/tests/data/schema_tests/merge_tests/basic_root.mediawiki b/tests/data/schema_tests/merge_tests/basic_root.mediawiki new file mode 100644 index 000000000..5b5f7a0e5 --- /dev/null +++ b/tests/data/schema_tests/merge_tests/basic_root.mediawiki @@ -0,0 +1,29 @@ +HED version="1.0.2" library="testlib" withStandard="8.2.0" unmerged="True" + +'''Prologue''' +This schema is the first official release that includes an xsd and requires unit class, unit modifier, value class, schema attribute and property sections. + +!# start schema + +'''Oboe-sound''' {rooted=Instrument-sound} + * Oboe-subsound + +'''Violin-sound''' {rooted=Instrument-sound} + * Violin-subsound + * Violin-subsound2 + + +!# end schema + +'''Unit classes''' + +'''Unit modifiers''' + +'''Value classes''' + +'''Schema attributes''' + +'''Properties''' +'''Epilogue''' + +!# end hed diff --git a/tests/data/schema_tests/merge_tests/basic_root.xml b/tests/data/schema_tests/merge_tests/basic_root.xml new file mode 100644 index 000000000..3f7f38dec --- /dev/null +++ b/tests/data/schema_tests/merge_tests/basic_root.xml @@ -0,0 +1,34 @@ + + + This schema is the first official release that includes an xsd and requires unit class, unit modifier, value class, schema attribute and property sections. + + + Oboe-sound + + rooted + Instrument-sound + + + Oboe-subsound + + + + Violin-sound + + rooted + Instrument-sound + + + Violin-subsound + + + Violin-subsound2 + + + + + + + + + diff --git a/tests/data/schema_tests/merge_tests/issues_tests/HED_badroot_0.0.1.mediawiki b/tests/data/schema_tests/merge_tests/issues_tests/HED_badroot_0.0.1.mediawiki index 0d76e15f8..a596775c1 100644 --- a/tests/data/schema_tests/merge_tests/issues_tests/HED_badroot_0.0.1.mediawiki +++ b/tests/data/schema_tests/merge_tests/issues_tests/HED_badroot_0.0.1.mediawiki @@ -5,7 +5,7 @@ This schema is the first official release that includes an xsd and requires unit !# start schema -'''NotRealTag'''{rooted} +'''NotRealTag'''{rooted=AlsoNotRealTag} * Oboe-sound diff --git a/tests/data/schema_tests/merge_tests/issues_tests/HED_dupesubroot_0.0.1.mediawiki b/tests/data/schema_tests/merge_tests/issues_tests/HED_dupesubroot_0.0.1.mediawiki index 63388f53c..672792aa8 100644 --- a/tests/data/schema_tests/merge_tests/issues_tests/HED_dupesubroot_0.0.1.mediawiki +++ b/tests/data/schema_tests/merge_tests/issues_tests/HED_dupesubroot_0.0.1.mediawiki @@ -5,16 +5,15 @@ This schema is the first official release that includes an xsd and requires unit !# start schema -'''Instrument-sound'''{rooted} -* Oboe-sound -** Oboe-subsound -* Violin-sound -** Violin-subsound -** Violin-subsound2 +'''Oboe-sound''' {rooted=Instrument-sound} +* Oboe-subsound +'''Violin-sound''' {rooted=Instrument-sound} +* Violin-subsound +* Violin-subsound2 -'''Instrument-sound'''{rooted} -* Oboe-sound + +'''Oboe-sound''' {rooted=Instrument-sound} !# end schema diff --git a/tests/data/schema_tests/merge_tests/issues_tests/HED_root_invalid1.mediawiki b/tests/data/schema_tests/merge_tests/issues_tests/HED_root_invalid1.mediawiki new file mode 100644 index 000000000..d5e6cf444 --- /dev/null +++ b/tests/data/schema_tests/merge_tests/issues_tests/HED_root_invalid1.mediawiki @@ -0,0 +1,18 @@ +HED library="testlib" version="1.0.2" + +'''Prologue''' +This schema is the first official release that includes an xsd and requires unit class, unit modifier, value class, schema attribute and property sections. + +!# start schema + +'''DummyTagToAvoidIssue''' + +'''Instrument-sound'''{rooted=DummyTagToAvoidIssue} +* Oboe-sound + + +!# end schema + +'''Epilogue''' + +!# end hed \ No newline at end of file diff --git a/tests/data/schema_tests/merge_tests/issues_tests/HED_root_invalid2.mediawiki b/tests/data/schema_tests/merge_tests/issues_tests/HED_root_invalid2.mediawiki new file mode 100644 index 000000000..979f72bde --- /dev/null +++ b/tests/data/schema_tests/merge_tests/issues_tests/HED_root_invalid2.mediawiki @@ -0,0 +1,18 @@ +HED library="testlib" version="1.0.2" withStandard="8.2.0" + +'''Prologue''' +This schema is the first official release that includes an xsd and requires unit class, unit modifier, value class, schema attribute and property sections. + +!# start schema + +'''DummyTagToAvoidIssue''' + +'''Instrument-sound'''{rooted=DummyTagToAvoidIssue} +* Oboe-sound + + +!# end schema + +'''Epilogue''' + +!# end hed \ No newline at end of file diff --git a/tests/data/schema_tests/merge_tests/issues_tests/HED_root_invalid3.mediawiki b/tests/data/schema_tests/merge_tests/issues_tests/HED_root_invalid3.mediawiki new file mode 100644 index 000000000..3438be07c --- /dev/null +++ b/tests/data/schema_tests/merge_tests/issues_tests/HED_root_invalid3.mediawiki @@ -0,0 +1,16 @@ +HED library="testlib" version="1.0.2" withStandard="8.2.0" unmerged="true" + +'''Prologue''' +This schema is the first official release that includes an xsd and requires unit class, unit modifier, value class, schema attribute and property sections. + +!# start schema + +'''Instrument-sound'''{rooted} +* Oboe-sound + + +!# end schema + +'''Epilogue''' + +!# end hed \ No newline at end of file diff --git a/tests/data/schema_tests/merge_tests/issues_tests/HED_root_wrong_place_0.0.1.mediawiki b/tests/data/schema_tests/merge_tests/issues_tests/HED_root_wrong_place_0.0.1.mediawiki index ec5dfd410..267a214e6 100644 --- a/tests/data/schema_tests/merge_tests/issues_tests/HED_root_wrong_place_0.0.1.mediawiki +++ b/tests/data/schema_tests/merge_tests/issues_tests/HED_root_wrong_place_0.0.1.mediawiki @@ -6,7 +6,7 @@ This schema is the first official release that includes an xsd and requires unit !# start schema '''Instrument-sound''' -* Oboe-sound {rooted} +* Oboe-sound {rooted=Instrument-sound} !# end schema diff --git a/tests/schema/test_hed_schema_io.py b/tests/schema/test_hed_schema_io.py index 299537e85..5511c7cd3 100644 --- a/tests/schema/test_hed_schema_io.py +++ b/tests/schema/test_hed_schema_io.py @@ -10,6 +10,7 @@ from hed.errors import HedExceptions +# todo: speed up these tests class TestHedSchema(unittest.TestCase): def test_load_invalid_schema(self): @@ -180,6 +181,9 @@ def _base_merging_test(self, files): # print(s1.filename) # print(s2.filename) self.assertTrue(result) + reload1 = load_schema(path1) + reload2 = load_schema(path2) + self.assertEqual(reload1, reload2) finally: os.remove(path1) os.remove(path2) @@ -189,6 +193,10 @@ def _base_merging_test(self, files): path2 = s2.save_as_mediawiki(save_merged=save_merged) result = filecmp.cmp(path1, path2) self.assertTrue(result) + + reload1 = load_schema(path1) + reload2 = load_schema(path2) + self.assertEqual(reload1, reload2) finally: os.remove(path1) os.remove(path2) @@ -212,6 +220,14 @@ def test_saving_merged(self): self._base_merging_test(files) + def test_saving_merged_rooted(self): + files = [ + load_schema(os.path.join(self.full_base_folder, "basic_root.mediawiki")), + load_schema(os.path.join(self.full_base_folder, "basic_root.xml")), + ] + + self._base_merging_test(files) + def _base_added_class_tests(self, schema): tag_entry = schema.all_tags["Modulator"] self.assertEqual(tag_entry.attributes["suggestedTag"], "Event") @@ -298,6 +314,10 @@ def test_cannot_load_schemas(self): files = [ os.path.join(self.full_base_folder, "issues_tests/HED_badroot_0.0.1.mediawiki"), os.path.join(self.full_base_folder, "issues_tests/HED_root_wrong_place_0.0.1.mediawiki"), + os.path.join(self.full_base_folder, "issues_tests/HED_root_invalid1.mediawiki"), + os.path.join(self.full_base_folder, "issues_tests/HED_root_invalid2.mediawiki"), + os.path.join(self.full_base_folder, "issues_tests/HED_root_invalid3.mediawiki"), + ] for file in files: with self.assertRaises(HedFileError):