Merge pull request #672 from IanCa/dev_change_rooted2

Change rooted implementation to new system

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)
 

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.
 

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)

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

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):

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",

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''' <nowiki>{rooted=Instrument-sound}</nowiki>
+	* Oboe-subsound
+
+'''Violin-sound''' <nowiki>{rooted=Instrument-sound}</nowiki>
+	* Violin-subsound
+	* Violin-subsound2
+
+
+!# end schema
+
+'''Unit classes'''
+
+'''Unit modifiers'''
+
+'''Value classes'''
+
+'''Schema attributes'''
+
+'''Properties'''
+'''Epilogue'''
+
+!# end hed

tests/data/schema_tests/merge_tests/basic_root.xml

@@ -0,0 +1,34 @@
+<?xml version="1.0" ?>
+<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.</prologue>
+   <schema>
+      <node>
+         <name>Oboe-sound</name>
+         <attribute>
+            <name>rooted</name>
+            <value>Instrument-sound</value>
+         </attribute>
+         <node>
+            <name>Oboe-subsound</name>
+         </node>
+      </node>
+      <node>
+         <name>Violin-sound</name>
+         <attribute>
+            <name>rooted</name>
+            <value>Instrument-sound</value>
+         </attribute>
+         <node>
+            <name>Violin-subsound</name>
+         </node>
+         <node>
+            <name>Violin-subsound2</name>
+         </node>
+      </node>
+   </schema>
+   <unitClassDefinitions/>
+   <unitModifierDefinitions/>
+   <valueClassDefinitions/>
+   <schemaAttributeDefinitions/>
+   <propertyDefinitions/>
+</HED>

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
 
 

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
 

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

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

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

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

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):