Remove custom combining character handling

Author: Enkidu93

machine/punctuation_analysis/quotation_mark_finder.py

@@ -43,8 +43,8 @@ def find_all_potential_quotation_marks_in_text_segment(
                 quotation_matches.append(
                     QuotationMarkStringMatch(
                         text_segment,
-                        text_segment.text.string_index_to_glyph_index(quotation_mark_match.start()),
-                        text_segment.text.string_index_to_glyph_index(quotation_mark_match.end()),
+                        quotation_mark_match.start(),
+                        quotation_mark_match.end(),
                     )
                 )
         return quotation_matches

machine/punctuation_analysis/text_segment.py

@@ -1,4 +1,3 @@
-import unicodedata
 from typing import Optional, Set
 
 from ..corpora.usfm_token import UsfmToken
@@ -7,7 +6,7 @@
 
 class TextSegment:
     def __init__(self):
-        self._text: GlyphString = GlyphString("")
+        self._text = ""
         self._immediate_preceding_marker: UsfmMarkerType = UsfmMarkerType.NO_MARKER
         self._markers_in_preceding_context: Set[UsfmMarkerType] = set()
         self.previous_segment: Optional[TextSegment] = None
@@ -32,7 +31,7 @@ def __eq__(self, value):
         return True
 
     @property
-    def text(self) -> "GlyphString":
+    def text(self) -> str:
         return self._text
 
     @property
@@ -55,7 +54,7 @@ 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 = GlyphString(self.substring_before(start_index) + replacement + self.substring_after(end_index))
+        self._text = self.substring_before(start_index) + replacement + self.substring_after(end_index)
         if self._usfm_token is not None:
             self._usfm_token.text = str(self._text)
 
@@ -77,70 +76,8 @@ def set_usfm_token(self, token: UsfmToken) -> "TextSegment.Builder":
             return self
 
         def set_text(self, text: str) -> "TextSegment.Builder":
-            self._text_segment._text = GlyphString(text)
+            self._text_segment._text = text
             return self
 
         def build(self) -> "TextSegment":
             return self._text_segment
-
-
-class GlyphString:
-    def __init__(self, string: str) -> None:
-        self._string = string
-        self._string_index_by_glyph_index = {
-            glyph_index: string_index
-            for glyph_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_glyph_index)
-
-    def __str__(self):
-        return self._string
-
-    def __eq__(self, other) -> bool:
-        if not isinstance(other, GlyphString):
-            return False
-        return self._string == other._string
-
-    def __getitem__(self, key) -> "GlyphString":
-        if isinstance(key, int):
-            glyph_start = self._normalize_start_index(key)
-            glyph_stop = self._normalize_stop_index(glyph_start + 1)
-            string_start = self._string_index_by_glyph_index.get(glyph_start, len(self))
-            string_stop = self._string_index_by_glyph_index.get(glyph_stop, None)
-            return GlyphString(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 _glyphString slices")
-            glyph_start = self._normalize_start_index(key.start)
-            glyph_stop = self._normalize_stop_index(key.stop)
-            string_start = self._string_index_by_glyph_index.get(glyph_start, len(self))
-            string_stop = self._string_index_by_glyph_index.get(glyph_stop, None)
-            return GlyphString(self._string[string_start:string_stop])
-        else:
-            raise TypeError("Indices must be integers or slices")
-
-    def _normalize_start_index(self, index: Optional[int]) -> int:
-        if index is None:
-            return 0
-        if index < 0:
-            return len(self) + index
-        return index
-
-    def _normalize_stop_index(self, index: Optional[int]) -> int:
-        if index is None:
-            return len(self)
-        if index < 0:
-            return len(self) + index
-        return index
-
-    def string_index_to_glyph_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_glyph_index.items():
-            if s_index == string_index:
-                return g_index
-        raise ValueError(f"No corresponding glyph index found for string index {string_index}.")

tests/corpora/test_quote_convention_changing_usfm_block_update_handler.py

@@ -476,13 +476,9 @@ 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 (
-        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"
+        quote_convention_changer._quotation_mark_finder.matches_to_return[1]._text_segment.text == "the test ends” here"
     )
 
 
@@ -497,7 +493,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 str(text_segments[0].text) == "test segment"
+    assert 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
@@ -520,7 +516,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 str(text_segments[0].text) == "test segment"
+    assert 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,
@@ -550,15 +546,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 str(text_segments[0].text) == "test segment1"
+    assert 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 str(text_segments[1].text) == "test segment2"
+    assert 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,
@@ -577,7 +573,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 str(segment.text) == "test segment"
+    assert 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
@@ -647,7 +643,7 @@ def test_update_quotation_marks() -> None:
 
     multi_char_to_single_char_quote_convention_changer._update_quotation_marks(multi_character_quotation_marks)
 
-    assert str(multi_character_text_segment.text) == "this “is ‘a test segment’ ”"
+    assert 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
@@ -707,7 +703,7 @@ def test_update_quotation_marks() -> None:
 
     single_char_to_multi_char_quote_convention_changer._update_quotation_marks(single_character_quotation_marks)
 
-    assert str(single_character_text_segment.text) == "this <<is <a test segment> >>"
+    assert 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
@@ -768,7 +764,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 str(segment.text) == ""
+    assert segment.text == ""
     assert UsfmMarkerType.EMBED not in segment._markers_in_preceding_context
     assert quote_convention_changer._verse_text_quotation_mark_resolver._issues == set()
 

tests/punctuation_analysis/test_quotation_mark_finder.py

@@ -177,7 +177,7 @@ 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)]
+    ) == [QuotationMarkStringMatch(TextSegment.Builder().set_text('उत्पत्ति "पुस्तकले').build(), 9, 10)]
 
 
 def test_that_it_uses_the_quote_convention_set() -> None:

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 str(quotation_mark_metadata.text_segment.text) == "He said to the woman, “Has God really said,"
+    assert 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 str(quotation_mark_metadata.text_segment.text) == "He said to the woman, «Has God really said,"
+    assert 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 str(quotation_mark_metadata.text_segment.text) == 'He said to the woman, "«as God really said,'
+    assert 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 str(quotation_mark_metadata.text_segment.text) == "He said to the woman, <<Has God really said,"
+    assert 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 str(quotation_mark_metadata.text_segment.text) == "He said to the woman, “Has God really said,"
+    assert 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
 

tests/punctuation_analysis/test_quotation_mark_string_match.py

@@ -129,7 +129,7 @@ def test_get_previous_character() -> None:
         0,
         1,
     )
-    assert quotation_mark_string_match.previous_character == "ले"
+    assert quotation_mark_string_match.previous_character == "\u0947"
 
 
 def test_get_next_character() -> None:

tests/punctuation_analysis/test_text_segment.py

@@ -5,7 +5,7 @@
 def test_builder_initialization() -> None:
     builder = TextSegment.Builder()
 
-    assert str(builder._text_segment.text) == ""
+    assert builder._text_segment.text == ""
     assert builder._text_segment.previous_segment is None
     assert builder._text_segment.next_segment is None
     assert builder._text_segment._immediate_preceding_marker is UsfmMarkerType.NO_MARKER
@@ -20,7 +20,7 @@ def test_builder_set_text() -> None:
     text = "Example text"
     builder.set_text(text)
 
-    assert str(builder._text_segment.text) == text
+    assert builder._text_segment.text == text
 
 
 def test_builder_set_previous_segment() -> None:
@@ -62,7 +62,7 @@ def test_builder_set_usfm_token() -> None:
     assert builder._text_segment._usfm_token is not None
     assert builder._text_segment._usfm_token.type == UsfmTokenType.TEXT
     assert builder._text_segment._usfm_token.text == "USFM token text"
-    assert str(builder._text_segment.text) == ""
+    assert builder._text_segment.text == ""
     assert builder._text_segment.previous_segment is None
     assert builder._text_segment.next_segment is None
 
@@ -148,10 +148,10 @@ def test_equals() -> None:
 
 def test_get_text() -> None:
     text_segment = TextSegment.Builder().set_text("example text").build()
-    assert str(text_segment.text) == "example text"
+    assert text_segment.text == "example text"
 
     text_segment = TextSegment.Builder().set_text("new example text").build()
-    assert str(text_segment.text) == "new example text"
+    assert text_segment.text == "new example text"
 
 
 def test_length() -> None:
@@ -163,7 +163,7 @@ def test_length() -> None:
 
     # Combining characters
     text_segment = TextSegment.Builder().set_text("उत्पत्ति पुस्तकले").build()
-    assert text_segment.length == 11
+    assert text_segment.length == 17
 
     # Surrogate pairs
     text_segment = TextSegment.Builder().set_text("𝜺𝜺").build()
@@ -251,28 +251,28 @@ def test_is_last_segment_in_verse() -> None:
 def test_replace_substring() -> None:
     text_segment = TextSegment.Builder().set_text("example text").build()
     text_segment.replace_substring(0, 7, "sample")
-    assert str(text_segment.text) == "sample text"
+    assert text_segment.text == "sample text"
 
     text_segment.replace_substring(7, 11, "text")
-    assert str(text_segment.text) == "sample text"
+    assert text_segment.text == "sample text"
 
     text_segment.replace_substring(0, 7, "")
-    assert str(text_segment.text) == "text"
+    assert text_segment.text == "text"
 
     text_segment.replace_substring(0, 4, "new'")
-    assert str(text_segment.text) == "new'"
+    assert text_segment.text == "new'"
 
     text_segment.replace_substring(3, 4, "\u2019")
-    assert str(text_segment.text) == "new\u2019"
+    assert text_segment.text == "new\u2019"
 
     text_segment.replace_substring(0, 0, "prefix ")
-    assert str(text_segment.text) == "prefix new\u2019"
+    assert text_segment.text == "prefix new\u2019"
 
     text_segment.replace_substring(0, 0, "")
-    assert str(text_segment.text) == "prefix new\u2019"
+    assert text_segment.text == "prefix new\u2019"
 
     text_segment.replace_substring(11, 11, " suffix")
-    assert str(text_segment.text) == "prefix new\u2019 suffix"
+    assert text_segment.text == "prefix new\u2019 suffix"
 
     text_segment.replace_substring(6, 6, "-")
-    assert str(text_segment.text) == "prefix- new\u2019 suffix"
+    assert text_segment.text == "prefix- new\u2019 suffix"