Port unicode string-related tests to Python; address discrepancy regarding combining characters in Python strings

Author: Enkidu93

machine/corpora/__init__.py

@@ -85,6 +85,7 @@
 from .usx_file_text_corpus import UsxFileTextCorpus
 from .usx_memory_text import UsxMemoryText
 from .usx_zip_text import UsxZipText
+from .zip_paratext_project_quote_convention_detector import ZipParatextProjectQuoteConventionDetector
 from .zip_paratext_project_settings_parser import ZipParatextProjectSettingsParser
 from .zip_paratext_project_settings_parser_base import ZipParatextProjectSettingsParserBase
 from .zip_paratext_project_terms_parser import ZipParatextProjectTermsParser
@@ -188,6 +189,7 @@
     "UsxFileTextCorpus",
     "UsxMemoryText",
     "UsxZipText",
+    "ZipParatextProjectQuoteConventionDetector",
     "ZipParatextProjectSettingsParser",
     "ZipParatextProjectSettingsParserBase",
     "ZipParatextProjectTermsParser",

machine/punctuation_analysis/quotation_mark_finder.py

@@ -36,11 +36,15 @@ def find_all_potential_quotation_marks_in_text_segment(
         self, text_segment: TextSegment
     ) -> List[QuotationMarkStringMatch]:
         quotation_matches: List[QuotationMarkStringMatch] = []
-        for quotation_mark_match in self._QUOTATION_MARK_PATTERN.finditer(text_segment.text):
+        for quotation_mark_match in self._QUOTATION_MARK_PATTERN.finditer(text_segment.text.string):
             if self._quote_conventions.is_valid_opening_quotation_mark(
                 quotation_mark_match.group()
             ) or self._quote_conventions.is_valid_closing_quotation_mark(quotation_mark_match.group()):
                 quotation_matches.append(
-                    QuotationMarkStringMatch(text_segment, quotation_mark_match.start(), quotation_mark_match.end())
+                    QuotationMarkStringMatch(
+                        text_segment,
+                        text_segment.text.string_index_to_grapheme_index(quotation_mark_match.start()),
+                        text_segment.text.string_index_to_grapheme_index(quotation_mark_match.end()),
+                    )
                 )
         return quotation_matches

machine/punctuation_analysis/quotation_mark_string_match.py

@@ -35,7 +35,7 @@ def __eq__(self, value):
 
     @property
     def quotation_mark(self) -> str:
-        return self._text_segment.text[self._start_index : self._end_index]
+        return self._text_segment.text[self._start_index : self._end_index].string
 
     def is_valid_opening_quotation_mark(self, quote_conventions: QuoteConventionSet) -> bool:
         return quote_conventions.is_valid_opening_quotation_mark(self.quotation_mark)
@@ -59,18 +59,18 @@ def previous_character(self) -> Optional[str]:
             if previous_segment is not None and not self._text_segment.marker_is_in_preceding_context(
                 UsfmMarkerType.PARAGRAPH
             ):
-                return previous_segment.text[-1]
+                return previous_segment.text[-1].string
             return None
-        return self._text_segment.text[self._start_index - 1]
+        return self._text_segment.text[self._start_index - 1].string
 
     @property
     def next_character(self) -> Optional[str]:
         if self.is_at_end_of_segment():
             next_segment = self._text_segment.next_segment
             if next_segment is not None and not next_segment.marker_is_in_preceding_context(UsfmMarkerType.PARAGRAPH):
-                return next_segment.text[0]
+                return next_segment.text[0].string
             return None
-        return self._text_segment.text[self._end_index]
+        return self._text_segment.text[self._end_index].string
 
     def leading_substring_matches(self, regex_pattern: regex.Pattern) -> bool:
         return regex_pattern.search(self._text_segment.substring_before(self._start_index)) is not None
@@ -102,7 +102,7 @@ def end_index(self) -> int:
     def context(self) -> str:
         return self._text_segment.text[
             max(self._start_index - 10, 0) : min(self._end_index + 10, len(self._text_segment.text))
-        ]
+        ].string
 
     def resolve(self, depth: int, direction: QuotationMarkDirection) -> QuotationMarkMetadata:
         return QuotationMarkMetadata(

machine/punctuation_analysis/text_segment.py

@@ -1,3 +1,4 @@
+import unicodedata
 from typing import Optional, Set
 
 from ..corpora.usfm_token import UsfmToken
@@ -6,7 +7,7 @@
 
 class TextSegment:
     def __init__(self):
-        self._text = ""
+        self._text: GraphemeString = GraphemeString("")
         self._immediate_preceding_marker: UsfmMarkerType = UsfmMarkerType.NO_MARKER
         self._markers_in_preceding_context: Set[UsfmMarkerType] = set()
         self.previous_segment: Optional[TextSegment] = None
@@ -31,18 +32,18 @@ def __eq__(self, value):
         return True
 
     @property
-    def text(self) -> str:
+    def text(self) -> "GraphemeString":
         return self._text
 
     @property
     def length(self) -> int:
         return len(self._text)
 
     def substring_before(self, index: int) -> str:
-        return self._text[:index]
+        return self._text[:index].string
 
     def substring_after(self, index: int) -> str:
-        return self._text[index:]
+        return self._text[index:].string
 
     def marker_is_in_preceding_context(self, marker: UsfmMarkerType) -> bool:
         return marker in self._markers_in_preceding_context
@@ -54,9 +55,9 @@ def is_last_segment_in_verse(self) -> bool:
         return self.index_in_verse == self.num_segments_in_verse - 1
 
     def replace_substring(self, start_index: int, end_index: int, replacement: str) -> None:
-        self._text = self.substring_before(start_index) + replacement + self.substring_after(end_index)
+        self._text = GraphemeString(self.substring_before(start_index) + replacement + self.substring_after(end_index))
         if self._usfm_token is not None:
-            self._usfm_token.text = self._text
+            self._usfm_token.text = self._text.string
 
     class Builder:
         def __init__(self):
@@ -76,8 +77,74 @@ def set_usfm_token(self, token: UsfmToken) -> "TextSegment.Builder":
             return self
 
         def set_text(self, text: str) -> "TextSegment.Builder":
-            self._text_segment._text = text
+            self._text_segment._text = GraphemeString(text)
             return self
 
         def build(self) -> "TextSegment":
             return self._text_segment
+
+
+class GraphemeString:
+    def __init__(self, string: str) -> None:
+        self._string = string
+        self._string_index_by_grapheme_index = {
+            grapheme_index: string_index
+            for grapheme_index, string_index in enumerate(
+                [i for i, c in enumerate(string) if unicodedata.category(c) not in ["Mc", "Mn"]]
+            )
+        }
+
+    def __len__(self) -> int:
+        return len(self._string_index_by_grapheme_index)
+
+    @property
+    def string(self) -> str:
+        return self._string
+
+    def __str__(self):
+        return self._string
+
+    def __eq__(self, other) -> bool:
+        if not isinstance(other, GraphemeString):
+            return False
+        return self._string == other.string
+
+    def __getitem__(self, key) -> "GraphemeString":
+        if isinstance(key, int):
+            grapheme_start = self._normalize_start_index(key)
+            grapheme_stop = self._normalize_stop_index(grapheme_start + 1)
+            string_start = self._string_index_by_grapheme_index.get(grapheme_start, len(self))
+            string_stop = self._string_index_by_grapheme_index.get(grapheme_stop, None)
+            return GraphemeString(self._string[string_start:string_stop])
+        elif isinstance(key, slice):
+            if key.step is not None and key.step != 1:
+                raise TypeError("Steps are not allowed in _GraphemeString slices")
+            grapheme_start = self._normalize_start_index(key.start)
+            grapheme_stop = self._normalize_stop_index(key.stop)
+            string_start = self._string_index_by_grapheme_index.get(grapheme_start, len(self))
+            string_stop = self._string_index_by_grapheme_index.get(grapheme_stop, None)
+            return GraphemeString(self._string[string_start:string_stop])
+        else:
+            raise TypeError("Indices must be integers or slices")
+
+    def _normalize_start_index(self, index: int | None) -> int:
+        if index is None:
+            return 0
+        if index < 0:
+            return len(self) + index
+        return index
+
+    def _normalize_stop_index(self, index: int | None) -> int:
+        if index is None:
+            return len(self)
+        if index < 0:
+            return len(self) + index
+        return index
+
+    def string_index_to_grapheme_index(self, string_index: int) -> int:
+        if string_index == len(self._string):
+            return len(self)
+        for g_index, s_index in self._string_index_by_grapheme_index.items():
+            if s_index == string_index:
+                return g_index
+        raise ValueError(f"No corresponding grapheme index found for string index {string_index}.")

tests/corpora/test_quote_convention_changing_usfm_block_update_handler.py

@@ -476,9 +476,12 @@ def test_process_scripture_element() -> None:
 
     assert quote_convention_changer._quotation_mark_finder.num_times_called == 1
     assert mock_quotation_mark_resolver.num_times_called == 1
-    assert quote_convention_changer._quotation_mark_finder.matches_to_return[0]._text_segment._text == "this is a ‘test"
     assert (
-        quote_convention_changer._quotation_mark_finder.matches_to_return[1]._text_segment._text
+        str(quote_convention_changer._quotation_mark_finder.matches_to_return[0]._text_segment.text)
+        == "this is a ‘test"
+    )
+    assert (
+        str(quote_convention_changer._quotation_mark_finder.matches_to_return[1]._text_segment.text)
         == "the test ends” here"
     )
 
@@ -494,7 +497,7 @@ def test_create_text_segments_basic() -> None:
     text_segments: List[TextSegment] = quote_convention_changer._create_text_segments(update_element)
 
     assert len(text_segments) == 1
-    assert text_segments[0]._text == "test segment"
+    assert str(text_segments[0].text) == "test segment"
     assert text_segments[0]._immediate_preceding_marker is UsfmMarkerType.NO_MARKER
     assert text_segments[0]._markers_in_preceding_context == set()
     assert text_segments[0].previous_segment is None
@@ -517,7 +520,7 @@ def test_create_text_segments_with_preceding_markers() -> None:
     text_segments: List[TextSegment] = quote_convention_changer._create_text_segments(update_element)
 
     assert len(text_segments) == 1
-    assert text_segments[0]._text == "test segment"
+    assert str(text_segments[0].text) == "test segment"
     assert text_segments[0]._immediate_preceding_marker == UsfmMarkerType.PARAGRAPH
     assert text_segments[0]._markers_in_preceding_context == {
         UsfmMarkerType.VERSE,
@@ -547,15 +550,15 @@ def test_create_text_segments_with_multiple_text_tokens() -> None:
     text_segments: List[TextSegment] = quote_convention_changer._create_text_segments(update_element)
 
     assert len(text_segments) == 2
-    assert text_segments[0]._text == "test segment1"
+    assert str(text_segments[0].text) == "test segment1"
     assert text_segments[0]._immediate_preceding_marker == UsfmMarkerType.PARAGRAPH
     assert text_segments[0]._markers_in_preceding_context == {
         UsfmMarkerType.VERSE,
         UsfmMarkerType.PARAGRAPH,
     }
     assert text_segments[0].previous_segment is None
     assert text_segments[0].next_segment == text_segments[1]
-    assert text_segments[1]._text == "test segment2"
+    assert str(text_segments[1].text) == "test segment2"
     assert text_segments[1]._immediate_preceding_marker == UsfmMarkerType.CHARACTER
     assert text_segments[1]._markers_in_preceding_context == {
         UsfmMarkerType.VERSE,
@@ -574,7 +577,7 @@ def test_create_text_segment() -> None:
     segment: Union[TextSegment, None] = quote_convention_changer._create_text_segment(usfm_token)
 
     assert segment is not None
-    assert segment._text == "test segment"
+    assert str(segment.text) == "test segment"
     assert segment._immediate_preceding_marker is UsfmMarkerType.NO_MARKER
     assert segment._markers_in_preceding_context == set()
     assert segment._usfm_token == usfm_token
@@ -644,7 +647,7 @@ def test_update_quotation_marks() -> None:
 
     multi_char_to_single_char_quote_convention_changer._update_quotation_marks(multi_character_quotation_marks)
 
-    assert multi_character_text_segment.text == "this “is ‘a test segment’ ”"
+    assert str(multi_character_text_segment.text) == "this “is ‘a test segment’ ”"
 
     assert multi_character_quotation_marks[0].start_index == 5
     assert multi_character_quotation_marks[0].end_index == 6
@@ -704,7 +707,7 @@ def test_update_quotation_marks() -> None:
 
     single_char_to_multi_char_quote_convention_changer._update_quotation_marks(single_character_quotation_marks)
 
-    assert single_character_text_segment.text == "this <<is <a test segment> >>"
+    assert str(single_character_text_segment.text) == "this <<is <a test segment> >>"
 
     assert single_character_quotation_marks[0].start_index == 5
     assert single_character_quotation_marks[0].end_index == 7
@@ -765,7 +768,7 @@ def test_start_new_chapter() -> None:
     segment = quote_convention_changer._next_scripture_text_segment_builder.build()
     assert quote_convention_changer._current_strategy == QuotationMarkUpdateStrategy.SKIP
     assert segment._immediate_preceding_marker == UsfmMarkerType.CHAPTER
-    assert segment._text == ""
+    assert str(segment.text) == ""
     assert UsfmMarkerType.EMBED not in segment._markers_in_preceding_context
     assert quote_convention_changer._verse_text_quotation_mark_resolver._issues == set()
 

tests/corpora/test_usfm_manual.py

@@ -5,7 +5,13 @@
 from typing import List, Optional
 
 import pytest
-from testutils.corpora_test_helpers import TEST_DATA_PATH, USFM_SOURCE_PROJECT_PATH, USFM_TARGET_PROJECT_PATH
+from testutils.corpora_test_helpers import (
+    TEST_DATA_PATH,
+    USFM_SOURCE_PROJECT_PATH,
+    USFM_SOURCE_PROJECT_ZIP_PATH,
+    USFM_TARGET_PROJECT_PATH,
+    USFM_TARGET_PROJECT_ZIP_PATH,
+)
 
 from machine.corpora import (
     FileParatextProjectSettingsParser,
@@ -15,9 +21,11 @@
     StandardParallelTextCorpus,
     UpdateUsfmRow,
     UpdateUsfmTextBehavior,
+    ZipParatextProjectQuoteConventionDetector,
     ZipParatextProjectSettingsParser,
     ZipParatextProjectTextUpdater,
 )
+from machine.punctuation_analysis import QuoteConventionDetector
 
 
 @pytest.mark.skip(reason="This is for manual testing only. Remove this decorator to run the test.")
@@ -124,3 +132,22 @@ def get_usfm(project_path: Path):
                 assert False, f"Failed to process {subdir}: {e}"
     else:
         get_usfm(PARATEXT_PROJECT_PATH)
+
+
+@pytest.mark.skip(reason="This is for manual testing only. Remove this decorator to run the test.")
+def test_analyze_corpora_quote_conventions():
+    source_handler = QuoteConventionDetector()
+    source_archive = zipfile.ZipFile(USFM_SOURCE_PROJECT_ZIP_PATH, "r")
+    source_quote_convention_detector = ZipParatextProjectQuoteConventionDetector(source_archive)
+    source_quote_convention_detector.get_quote_convention_analysis(source_handler)
+
+    target_handler = QuoteConventionDetector()
+    target_archive = zipfile.ZipFile(USFM_TARGET_PROJECT_ZIP_PATH, "r")
+    target_quote_convention_detector = ZipParatextProjectQuoteConventionDetector(target_archive)
+    target_quote_convention_detector.get_quote_convention_analysis(target_handler)
+
+    source_analysis = source_handler.detect_quote_convention()
+    target_analysis = target_handler.detect_quote_convention()
+
+    assert source_analysis is not None
+    assert target_analysis is not None

tests/punctuation_analysis/test_quotation_mark_finder.py

@@ -175,6 +175,10 @@ def test_that_all_possible_quotation_marks_are_identified() -> None:
         ),
     ]
 
+    assert quotation_mark_finder.find_all_potential_quotation_marks_in_text_segment(
+        TextSegment.Builder().set_text('उत्पत्ति "पुस्तकले').build()
+    ) == [QuotationMarkStringMatch(TextSegment.Builder().set_text('उत्पत्ति "पुस्तकले').build(), 6, 7)]
+
 
 def test_that_it_uses_the_quote_convention_set() -> None:
     standard_english_quote_convention = STANDARD_QUOTE_CONVENTIONS.get_quote_convention_by_name("standard_english")

tests/punctuation_analysis/test_quotation_mark_metadata.py

@@ -19,7 +19,7 @@ def test_update_quotation_mark() -> None:
         end_index=23,
     )
     quotation_mark_metadata.update_quotation_mark(get_quote_convention_by_name("standard_english"))
-    assert quotation_mark_metadata.text_segment._text == "He said to the woman, “Has God really said,"
+    assert str(quotation_mark_metadata.text_segment.text) == "He said to the woman, “Has God really said,"
 
     quotation_mark_metadata = QuotationMarkMetadata(
         quotation_mark='"',
@@ -30,7 +30,7 @@ def test_update_quotation_mark() -> None:
         end_index=23,
     )
     quotation_mark_metadata.update_quotation_mark(get_quote_convention_by_name("western_european"))
-    assert quotation_mark_metadata.text_segment._text == "He said to the woman, «Has God really said,"
+    assert str(quotation_mark_metadata.text_segment.text) == "He said to the woman, «Has God really said,"
 
     quotation_mark_metadata = QuotationMarkMetadata(
         quotation_mark='"',
@@ -41,7 +41,7 @@ def test_update_quotation_mark() -> None:
         end_index=24,
     )
     quotation_mark_metadata.update_quotation_mark(get_quote_convention_by_name("western_european"))
-    assert quotation_mark_metadata.text_segment._text == 'He said to the woman, "«as God really said,'
+    assert str(quotation_mark_metadata.text_segment.text) == 'He said to the woman, "«as God really said,'
 
 
 def test_update_quotation_mark_with_multi_character_quotation_marks() -> None:
@@ -54,7 +54,7 @@ def test_update_quotation_mark_with_multi_character_quotation_marks() -> None:
         end_index=23,
     )
     quotation_mark_metadata.update_quotation_mark(get_quote_convention_by_name("typewriter_french"))
-    assert quotation_mark_metadata.text_segment._text == "He said to the woman, <<Has God really said,"
+    assert str(quotation_mark_metadata.text_segment.text) == "He said to the woman, <<Has God really said,"
     assert quotation_mark_metadata.start_index == 22
     assert quotation_mark_metadata.end_index == 24
 
@@ -67,7 +67,7 @@ def test_update_quotation_mark_with_multi_character_quotation_marks() -> None:
         end_index=24,
     )
     quotation_mark_metadata.update_quotation_mark(get_quote_convention_by_name("standard_english"))
-    assert quotation_mark_metadata.text_segment._text == "He said to the woman, “Has God really said,"
+    assert str(quotation_mark_metadata.text_segment.text) == "He said to the woman, “Has God really said,"
     assert quotation_mark_metadata.start_index == 22
     assert quotation_mark_metadata.end_index == 23