Repository: ocr-d/core
Issue: page_validator.py produces wrong concatenated text

In [get_text()](https://github.com/OCR-D/core/blob/master/ocrd_validators/ocrd_validators/page_validator.py#L260), the `TextEquiv` with `index=1` is used if it exists. The way I read the documentation of the `index` attribute in the [PAGE schema](https://www.primaresearch.org/schema/PAGE/gts/pagecontent/2019-07-15/pagecontent.xsd), it should use the one with the lowest index:

>  Used for sort order in case multiple TextEquivs are defined. The text content with the lowest index should be interpreted as the main text content. 

The lowest possible value for `index` is `0`, according to the schema.

Changed files:
ocrd/ocrd/cli/validate.py, tests/validator/test_page_validator.py, ocrd_validators/ocrd_validators/page_validator.py

diff --git a/ocrd/ocrd/cli/validate.py b/ocrd/ocrd/cli/validate.py
index b009559b..ed2aea39 100644
--- a/ocrd/ocrd/cli/validate.py
+++ b/ocrd/ocrd/cli/validate.py
@@ -57,7 +57,7 @@ def validate_parameters(ocrd_tool, executable, param_json):
 @validate_cli.command('page')
 @click.argument('page', required=True, nargs=1)
 @click.option('--page-textequiv-consistency', help="How strict to check PAGE multi-level textequiv consistency", type=click.Choice(['strict', 'lax', 'fix', 'off']), default='strict')
-@click.option('--page-textequiv-strategy', help="Strategy to determine the correct textequiv", type=click.Choice(['index1']), default='index1')
+@click.option('--page-textequiv-strategy', help="Strategy to determine the correct textequiv", type=click.Choice(['first']), default='first')
 @click.option('--check-baseline', help="Whether Baseline must be fully within TextLine/Coords", is_flag=True, default=False)
 @click.option('--check-coords', help="Whether *Region/TextLine/Word/Glyph must each be fully contained within Border/*Region/TextLine/Word, resp.", is_flag=True, default=False)
 def validate_page(page, **kwargs):
diff --git a/ocrd_validators/ocrd_validators/page_validator.py b/ocrd_validators/ocrd_validators/page_validator.py
index 1b1eac7f..d2008029 100644
--- a/ocrd_validators/ocrd_validators/page_validator.py
+++ b/ocrd_validators/ocrd_validators/page_validator.py
@@ -257,37 +257,68 @@ def concatenate(nodes, concatenate_with, page_textequiv_strategy):
     tokens = [get_text(node, page_textequiv_strategy) for node in nodes]
     return concatenate_with.join(tokens).strip()
 
-def get_text(node, page_textequiv_strategy):
+def get_text(node, page_textequiv_strategy='first'):
     """
-    Get the most confident text results, either those with @index = 1 or the first text results or empty string.
+    Get the first or most confident among text results (depending on ``page_textequiv_strategy``).
+    For the strategy ``best``, return the string of the highest scoring result.
+    For the strategy ``first``, return the string of the lowest indexed result.
+    If there are no scores/indexes, use the first result.
+    If there are no results, return the empty string.
     """
     textEquivs = node.get_TextEquiv()
     if not textEquivs:
         log.debug("No text results on %s %s", node, node.id)
         return ''
-    #  elif page_textequiv_strategy == 'index1':
+    elif page_textequiv_strategy == 'best':
+        if len(textEquivs) > 1:
+            textEquivsSorted = sorted([x for x in textEquivs if x.conf],
+                                      # generateDS does not convert simpleType for attributes (yet?)
+                                      key=lambda x: float(x.conf))
+            if textEquivsSorted:
+                return textEquivsSorted[-1].get_Unicode().strip()
+        # fall back to first element
+        return textEquivs[0].get_Unicode().strip()
+    #elif page_textequiv_strategy == 'first':
     else:
         if len(textEquivs) > 1:
-            index1 = [x for x in textEquivs if x.index == 1]
-            if index1:
-                return index1[0].get_Unicode().strip()
+            textEquivsSorted = sorted([x for x in textEquivs if isinstance(x.index, int)],
+                                      key=lambda x: x.index)
+            if textEquivsSorted:
+                return textEquivsSorted[0].get_Unicode().strip()
+        # fall back to first element
         return textEquivs[0].get_Unicode().strip()
 
 def set_text(node, text, page_textequiv_strategy):
     """
-    Set the most confident text results, either those with @index = 1, the first text results or add new one.
+    Set the first or most confident among text results (depending on ``page_textequiv_strategy``).
+    For the strategy ``best``, set the string of the highest scoring result.
+    For the strategy ``first``, set the string of the lowest indexed result.
+    If there are no scores/indexes, use the first result.
+    If there are no results, add a new one.
     """
     text = text.strip()
     textEquivs = node.get_TextEquiv()
     if not textEquivs:
-        node.add_TextEquiv(TextEquivType(Unicode=text))
-    #  elif page_textequiv_strategy == 'index1':
+        node.add_TextEquiv(TextEquivType(Unicode=text)) # or index=0 ?
+    elif page_textequiv_strategy == 'best':
+        if len(textEquivs) > 1:
+            textEquivsSorted = sorted([x for x in textEquivs if x.conf],
+                                      # generateDS does not convert simpleType for attributes (yet?)
+                                      key=lambda x: float(x.conf))
+            if textEquivsSorted:
+                textEquivsSorted[-1].set_Unicode(text)
+                return
+        # fall back to first element
+        textEquivs[0].set_Unicode(text)
+    #elif page_textequiv_strategy == 'first':
     else:
         if len(textEquivs) > 1:
-            index1 = [x for x in textEquivs if x.index == 1]
-            if index1:
-                index1[0].set_Unicode(text)
+            textEquivsSorted = sorted([x for x in textEquivs if isinstance(x.index, int)],
+                                      key=lambda x: x.index)
+            if textEquivsSorted:
+                textEquivsSorted[0].set_Unicode(text)
                 return
+        # fall back to first element
         textEquivs[0].set_Unicode(text)
 
 class PageValidator():
@@ -299,7 +330,7 @@ class PageValidator():
     @deprecated_alias(strictness='page_textequiv_consistency')
     @deprecated_alias(strategy='page_textequiv_strategy')
     def validate(filename=None, ocrd_page=None, ocrd_file=None,
-                 page_textequiv_consistency='strict', page_textequiv_strategy='index1',
+                 page_textequiv_consistency='strict', page_textequiv_strategy='first',
                  check_baseline=True, check_coords=True):
         """
         Validates a PAGE file for consistency by filename, OcrdFile or passing OcrdPage directly.
@@ -309,7 +340,7 @@ class PageValidator():
             ocrd_page (OcrdPage): OcrdPage instance
             ocrd_file (OcrdFile): OcrdFile instance wrapping OcrdPage
             page_textequiv_consistency (string): 'strict', 'lax', 'fix' or 'off'
-            page_textequiv_strategy (string): Currently only 'index1'
+            page_textequiv_strategy (string): Currently only 'first'
             check_baseline (bool): whether Baseline must be fully within TextLine/Coords
             check_coords (bool): whether *Region/TextLine/Word/Glyph must each be fully
                                  contained within Border/*Region/TextLine/Word, resp.
@@ -328,7 +359,7 @@ class PageValidator():
             file_id = filename
         else:
             raise Exception("At least one of ocrd_page, ocrd_file or filename must be set")
-        if page_textequiv_strategy not in ('index1'):
+        if page_textequiv_strategy not in ('first'):
             raise Exception("page_textequiv_strategy %s not implemented" % page_textequiv_strategy)
         if page_textequiv_consistency not in ('strict', 'lax', 'fix', 'off'):
             raise Exception("page_textequiv_consistency level %s not implemented" % page_textequiv_consistency)
diff --git a/tests/validator/test_page_validator.py b/tests/validator/test_page_validator.py
index 8a0fcbcf..4a1dddde 100644
--- a/tests/validator/test_page_validator.py
+++ b/tests/validator/test_page_validator.py
@@ -21,7 +21,7 @@ class TestPageValidator(TestCase):
         with self.assertRaisesRegex(Exception, 'page_textequiv_strategy best not implemented'):
             PageValidator.validate(filename=FAULTY_GLYPH_PAGE_FILENAME, strategy='best')
         with self.assertRaisesRegex(Exception, 'page_textequiv_consistency level superstrictest not implemented'):
-            PageValidator.validate(filename=FAULTY_GLYPH_PAGE_FILENAME, page_textequiv_consistency='superstrictest', strategy='index1')
+            PageValidator.validate(filename=FAULTY_GLYPH_PAGE_FILENAME, page_textequiv_consistency='superstrictest', strategy='first')
 
     def test_validate_filename(self):
         report = PageValidator.validate(filename=FAULTY_GLYPH_PAGE_FILENAME)
@@ -50,7 +50,7 @@ class TestPageValidator(TestCase):
         report = PageValidator.validate(ocrd_page=ocrd_page, strictness='lax')
         self.assertEqual(len([e for e in report.errors if isinstance(e, ConsistencyError)]), 1, '1 textequiv consistency errors - lax')
 
-    def test_validate_multi_textequiv_index1(self):
+    def test_validate_multi_textequiv_first(self):
         ocrd_page = parse(assets.path_to('kant_aufklaerung_1784/data/OCR-D-GT-PAGE/PAGE_0020_PAGE.xml'), silence=True)
         report = PageValidator.validate(ocrd_page=ocrd_page)
         self.assertEqual(len([e for e in report.errors if isinstance(e, ConsistencyError)]), 25, '25 textequiv consistency errors - strict')
@@ -58,15 +58,15 @@ class TestPageValidator(TestCase):
         word = ocrd_page.get_Page().get_TextRegion()[0].get_TextLine()[0].get_Word()[1]
 
         # delete all textequivs
-        del(word.get_TextEquiv()[0])
+        word.set_TextEquiv([])
 
         # Add textequiv
-        set_text(word, 'FOO', 'index1')
+        set_text(word, 'FOO', 'first')
         word.add_TextEquiv(TextEquivType(Unicode='BAR', conf=.7))
-        word.add_TextEquiv(TextEquivType(Unicode='BAZ', conf=.5, index=1))
-        self.assertEqual(get_text(word, 'index1'), 'BAZ')
-        set_text(word, 'XYZ', 'index1')
-        self.assertEqual(get_text(word, 'index1'), 'XYZ')
+        word.add_TextEquiv(TextEquivType(Unicode='BAZ', conf=.5, index=0))
+        self.assertEqual(get_text(word, 'first'), 'BAZ')
+        set_text(word, 'XYZ', 'first')
+        self.assertEqual(get_text(word, 'first'), 'XYZ')
 
 
     def test_validate_multi_textequiv(self):