diff --git a/machine/corpora/__init__.py b/machine/corpora/__init__.py index c4ca4702..20f3a1da 100644 --- a/machine/corpora/__init__.py +++ b/machine/corpora/__init__.py @@ -59,12 +59,8 @@ normalize, unescape_spaces, ) -from .update_usfm_parser_handler import ( - UpdateUsfmMarkerBehavior, - UpdateUsfmParserHandler, - UpdateUsfmRow, - UpdateUsfmTextBehavior, -) +from .update_usfm_behavior import UpdateUsfmMarkerBehavior, UpdateUsfmTextBehavior +from .update_usfm_parser_handler import UpdateUsfmParserHandler, UpdateUsfmRow from .usfm_file_text import UsfmFileText from .usfm_file_text_corpus import UsfmFileTextCorpus from .usfm_memory_text import UsfmMemoryText diff --git a/machine/corpora/paratext_project_text_updater_base.py b/machine/corpora/paratext_project_text_updater_base.py index 7eee05fc..188f31c7 100644 --- a/machine/corpora/paratext_project_text_updater_base.py +++ b/machine/corpora/paratext_project_text_updater_base.py @@ -5,12 +5,8 @@ from .paratext_project_file_handler import ParatextProjectFileHandler from .paratext_project_settings import ParatextProjectSettings from .paratext_project_settings_parser_base import ParatextProjectSettingsParserBase -from .update_usfm_parser_handler import ( - UpdateUsfmMarkerBehavior, - UpdateUsfmParserHandler, - UpdateUsfmRow, - UpdateUsfmTextBehavior, -) +from .update_usfm_behavior import UpdateUsfmMarkerBehavior, UpdateUsfmTextBehavior +from .update_usfm_parser_handler import UpdateUsfmParserHandler, UpdateUsfmRow from .usfm_parser import parse_usfm from .usfm_token import UsfmTokenType from .usfm_tokenizer import UsfmToken, UsfmTokenizer diff --git a/machine/corpora/place_markers_usfm_update_block_handler.py b/machine/corpora/place_markers_usfm_update_block_handler.py index 287eb2d6..421d8b2b 100644 --- a/machine/corpora/place_markers_usfm_update_block_handler.py +++ b/machine/corpora/place_markers_usfm_update_block_handler.py @@ -4,7 +4,7 @@ from ..translation.word_alignment_matrix import WordAlignmentMatrix from .segment_boundary_adjuster import SegmentBoundaryAdjuster -from .update_usfm_parser_handler import UpdateUsfmMarkerBehavior +from .update_usfm_behavior import UpdateUsfmMarkerBehavior from .usfm_token import UsfmToken, UsfmTokenType from .usfm_update_block import UsfmUpdateBlock from .usfm_update_block_element import UsfmUpdateBlockElement, UsfmUpdateBlockElementType @@ -39,18 +39,7 @@ def process_block(self, block: UsfmUpdateBlock) -> UsfmUpdateBlock: or alignment_info["alignment"].row_count == 0 or alignment_info["alignment"].column_count == 0 or not any( - ( - ( - e.type == UsfmUpdateBlockElementType.PARAGRAPH - and alignment_info["paragraph_behavior"] == UpdateUsfmMarkerBehavior.PRESERVE - and len(e.tokens) == 1 - ) - or ( - e.type == UsfmUpdateBlockElementType.STYLE - and alignment_info["style_behavior"] == UpdateUsfmMarkerBehavior.PRESERVE - ) - ) - for e in elements + e.is_placeable(alignment_info["paragraph_behavior"], alignment_info["style_behavior"]) for e in elements ) ): return block @@ -74,7 +63,8 @@ def process_block(self, block: UsfmUpdateBlock) -> UsfmUpdateBlock: elements.pop(i) elif not ( element.type == UsfmUpdateBlockElementType.EMBED - or (element.type == UsfmUpdateBlockElementType.TEXT and len(element.tokens[0].to_usfm().strip()) == 0) + or element.type == UsfmUpdateBlockElementType.OTHER + or (element.type == UsfmUpdateBlockElementType.TEXT and len(element.get_text().strip()) == 0) ): eob_empty_paras = False @@ -92,7 +82,7 @@ def process_block(self, block: UsfmUpdateBlock) -> UsfmUpdateBlock: for element in elements: if element.type == UsfmUpdateBlockElementType.TEXT: if element.marked_for_removal: - text = element.tokens[0].to_usfm() + text = element.get_text() src_sent += text # Track seen tokens @@ -103,7 +93,7 @@ def process_block(self, block: UsfmUpdateBlock) -> UsfmUpdateBlock: if len(text.strip()) > 0: src_tok_idx += 1 else: - trg_sent += element.tokens[0].to_usfm() + trg_sent += element.get_text() if element.marked_for_removal or ( element.type == UsfmUpdateBlockElementType.PARAGRAPH @@ -163,6 +153,10 @@ def process_block(self, block: UsfmUpdateBlock) -> UsfmUpdateBlock: to_insert.sort(key=lambda x: x[0]) to_insert += [(len(trg_sent), element) for element in embed_elements + end_elements] + # In the case of unclosed markers, to_insert might be empty + if len(to_insert) == 0: + return block + # Construct new text tokens to put between markers # and reincorporate headers and empty end-of-verse paragraph markers if to_insert[0][0] > 0: diff --git a/machine/corpora/update_usfm_behavior.py b/machine/corpora/update_usfm_behavior.py new file mode 100644 index 00000000..ed172cf4 --- /dev/null +++ b/machine/corpora/update_usfm_behavior.py @@ -0,0 +1,12 @@ +from enum import Enum, auto + + +class UpdateUsfmTextBehavior(Enum): + PREFER_EXISTING = auto() + PREFER_NEW = auto() + STRIP_EXISTING = auto() + + +class UpdateUsfmMarkerBehavior(Enum): + PRESERVE = auto() + STRIP = auto() diff --git a/machine/corpora/update_usfm_parser_handler.py b/machine/corpora/update_usfm_parser_handler.py index c21f7cf8..87689d8a 100644 --- a/machine/corpora/update_usfm_parser_handler.py +++ b/machine/corpora/update_usfm_parser_handler.py @@ -1,9 +1,9 @@ -from enum import Enum, auto from typing import Callable, Dict, Iterable, List, Optional, Sequence, Tuple, Union from ..scripture.verse_ref import IgnoreSegmentsVerseRef, VerseRef, Versification from .scripture_ref import ScriptureRef from .scripture_ref_usfm_parser_handler_base import ScriptureRefUsfmParserHandlerBase, ScriptureTextType +from .update_usfm_behavior import UpdateUsfmMarkerBehavior, UpdateUsfmTextBehavior from .usfm_parser_state import UsfmParserState from .usfm_stylesheet import UsfmStylesheet from .usfm_tag import UsfmTextType @@ -14,17 +14,6 @@ from .usfm_update_block_handler import UsfmUpdateBlockHandler, UsfmUpdateBlockHandlerError -class UpdateUsfmTextBehavior(Enum): - PREFER_EXISTING = auto() - PREFER_NEW = auto() - STRIP_EXISTING = auto() - - -class UpdateUsfmMarkerBehavior(Enum): - PRESERVE = auto() - STRIP = auto() - - class _RowInfo: def __init__(self, row_index: int): self.row_index = row_index @@ -235,7 +224,11 @@ def start_note(self, state: UsfmParserState, marker: str, caller: str, category: def end_note(self, state: UsfmParserState, marker: str, closed: bool) -> None: if closed: - self._collect_updatable_tokens(state) + # Mirror start_note: an embed in a duplicate verse is dropped, end marker included. + if self._duplicate_verse: + self._skip_updatable_tokens(state) + else: + self._collect_updatable_tokens(state) super().end_note(state, marker, closed) @@ -264,14 +257,18 @@ def end_char( attributes: Sequence[UsfmAttribute], closed: bool, ) -> None: - if self._current_text_type == ScriptureTextType.EMBED: - self._collect_updatable_tokens(state) - else: - self._replace_with_new_tokens(state) - if self._style_behavior == UpdateUsfmMarkerBehavior.STRIP: - self._skip_updatable_tokens(state) - else: + # An implicitly closed character style has no end marker of its own, so the token at + # state.index belongs to whatever closed it (e.g. the next paragraph marker). Leave it + # for the callback that handles it, as end_note and end_sidebar already do. + if closed: + if self._current_text_type == ScriptureTextType.EMBED: self._collect_updatable_tokens(state) + else: + self._replace_with_new_tokens(state) + if self._style_behavior == UpdateUsfmMarkerBehavior.STRIP: + self._skip_updatable_tokens(state) + else: + self._collect_updatable_tokens(state) super().end_char(state, marker, attributes, closed) diff --git a/machine/corpora/usfm_update_block_element.py b/machine/corpora/usfm_update_block_element.py index 7a4c01c6..045002ff 100644 --- a/machine/corpora/usfm_update_block_element.py +++ b/machine/corpora/usfm_update_block_element.py @@ -1,6 +1,7 @@ from dataclasses import dataclass from enum import Enum, auto +from .update_usfm_behavior import UpdateUsfmMarkerBehavior from .usfm_token import UsfmToken @@ -22,3 +23,17 @@ def get_tokens(self) -> list[UsfmToken]: if self.marked_for_removal: return [] return self.tokens.copy() + + def get_text(self) -> str: + return "".join(t.to_usfm() for t in self.tokens) + + def is_placeable( + self, paragraph_behavior: UpdateUsfmMarkerBehavior, style_behavior: UpdateUsfmMarkerBehavior + ) -> bool: + if self.marked_for_removal: + return False + if self.type == UsfmUpdateBlockElementType.PARAGRAPH: + return paragraph_behavior == UpdateUsfmMarkerBehavior.PRESERVE and len(self.tokens) == 1 + if self.type == UsfmUpdateBlockElementType.STYLE: + return style_behavior == UpdateUsfmMarkerBehavior.PRESERVE + return False diff --git a/tests/corpora/test_place_markers_usfm_update_block_handler.py b/tests/corpora/test_place_markers_usfm_update_block_handler.py index 41eff43a..b0d99a91 100644 --- a/tests/corpora/test_place_markers_usfm_update_block_handler.py +++ b/tests/corpora/test_place_markers_usfm_update_block_handler.py @@ -729,6 +729,176 @@ def test_adjustment_of_placed_paragraph_marker() -> None: assert_usfm_equals(target, result) +def test_unclosed_style_marker_in_non_verse_paragraph() -> None: + # An unclosed character style is closed implicitly by the next paragraph marker, which must + # not be pulled into the block being closed (and then dropped as part of the removed style). + source = "(A)" + pretranslation = "(A translated)" + align_info = PlaceMarkersAlignmentInfo( + source_tokens=[t for t in TOKENIZER.tokenize(source)], + translation_tokens=[t for t in TOKENIZER.tokenize(pretranslation)], + alignment=to_word_alignment_matrix("0-0 1-1 2-2"), + paragraph_behavior=UpdateUsfmMarkerBehavior.PRESERVE, + style_behavior=UpdateUsfmMarkerBehavior.STRIP, + ) + rows = [ + UpdateUsfmRow(scr_ref("PSA 119:0/1:d"), str(pretranslation), metadata={"alignment_info": align_info}), + UpdateUsfmRow(scr_ref("PSA 119:1"), "New verse 1"), + ] + usfm = r"""\id PSA +\c 119 +\d \bd (A) +\q1 +\v 1 Verse 1 +""" + + target = update_usfm(rows, usfm, update_block_handlers=[PlaceMarkersUsfmUpdateBlockHandler()]) + result = r"""\id PSA +\c 119 +\d (A translated) +\q1 +\v 1 New verse 1 +""" + assert_usfm_equals(target, result) + + +def test_unmatched_end_marker() -> None: + # A stray end marker has no matching start marker, so it is marked for removal even when + # styles are preserved. It must not be mistaken for a marker that can be placed. + source = "Section header" + pretranslation = "New section header" + align_info = PlaceMarkersAlignmentInfo( + source_tokens=[t for t in TOKENIZER.tokenize(source)], + translation_tokens=[t for t in TOKENIZER.tokenize(pretranslation)], + alignment=to_word_alignment_matrix("0-1 1-2"), + paragraph_behavior=UpdateUsfmMarkerBehavior.PRESERVE, + style_behavior=UpdateUsfmMarkerBehavior.PRESERVE, + ) + rows = [ + UpdateUsfmRow(scr_ref("MAT 1:0/1:s"), str(pretranslation), metadata={"alignment_info": align_info}), + UpdateUsfmRow(scr_ref("MAT 1:1"), "New verse 1"), + ] + usfm = r"""\id MAT +\c 1 +\s Section header\it* +\p +\v 1 Verse 1 +""" + + target = update_usfm( + rows, + usfm, + style_behavior=UpdateUsfmMarkerBehavior.PRESERVE, + update_block_handlers=[PlaceMarkersUsfmUpdateBlockHandler()], + ) + result = r"""\id MAT +\c 1 +\s New section header +\p +\v 1 New verse 1 +""" + assert_usfm_equals(target, result) + + +def test_marker_behavior_disagrees_with_alignment_info() -> None: + # The behaviors in the alignment info are supplied by the caller and can disagree with the + # ones the updater was built with. The markers are already stripped, so there is nothing to + # place and the block is left alone. + source = "Section header" + pretranslation = "New section header" + align_info = PlaceMarkersAlignmentInfo( + source_tokens=[t for t in TOKENIZER.tokenize(source)], + translation_tokens=[t for t in TOKENIZER.tokenize(pretranslation)], + alignment=to_word_alignment_matrix("0-1 1-2"), + paragraph_behavior=UpdateUsfmMarkerBehavior.PRESERVE, + style_behavior=UpdateUsfmMarkerBehavior.PRESERVE, + ) + rows = [ + UpdateUsfmRow(scr_ref("MAT 1:0/1:s"), str(pretranslation), metadata={"alignment_info": align_info}), + UpdateUsfmRow(scr_ref("MAT 1:1"), "New verse 1"), + ] + usfm = r"""\id MAT +\c 1 +\s Section \it header\it* +\p +\v 1 Verse 1 +""" + + target = update_usfm( + rows, + usfm, + style_behavior=UpdateUsfmMarkerBehavior.STRIP, + update_block_handlers=[PlaceMarkersUsfmUpdateBlockHandler()], + ) + result = r"""\id MAT +\c 1 +\s New section header +\p +\v 1 New verse 1 +""" + assert_usfm_equals(target, result) + + +def test_other_elements_do_not_affect_embed_placement() -> None: + # Attributes, milestones and the like are never transferred, so their presence must not + # change where anything else lands - here, an embed before end-of-verse paragraph markers. + source = "This is the first part. This is the second part." + pretranslation = "Esta es la primera parte. Esta es la segunda parte." + result = r"""\id MAT +\c 1 +\v 1 Esta es la primera parte. Esta es la segunda parte. \f + \ft Footnote\f* +\q1 +\q2 +""" + for milestone in ["", r" \ts-s\*"]: + align_info = PlaceMarkersAlignmentInfo( + source_tokens=[t for t in TOKENIZER.tokenize(source)], + translation_tokens=[t for t in TOKENIZER.tokenize(pretranslation)], + alignment=to_word_alignment_matrix("0-0 1-1 2-2 3-3 4-4 5-5 6-6 7-7 8-8 9-9 10-10"), + paragraph_behavior=UpdateUsfmMarkerBehavior.PRESERVE, + style_behavior=UpdateUsfmMarkerBehavior.STRIP, + ) + rows = [UpdateUsfmRow(scr_ref("MAT 1:1"), str(pretranslation), metadata={"alignment_info": align_info})] + usfm = ( + "\\id MAT\n\\c 1\n" + "\\v 1 This is the first part. This is the second part.\\f + \\ft Footnote\\f*\n" + f"\\q1{milestone}\n\\q2\n" + ) + + target = update_usfm(rows, usfm, update_block_handlers=[PlaceMarkersUsfmUpdateBlockHandler()]) + assert_usfm_equals(target, result) + + +def test_multiple_text_rows_in_verse_ranges_are_updated() -> None: + # Verse ranges contain multiple text rows, which must be processed as if they were a single row + source = "This is the first part. This is the second part." + pretranslation = "Esta es la primera parte. Esta es la segunda parte." + align_info = PlaceMarkersAlignmentInfo( + source_tokens=[t for t in TOKENIZER.tokenize(source)], + translation_tokens=[t for t in TOKENIZER.tokenize(pretranslation)], + alignment=to_word_alignment_matrix("0-0 1-1 2-2 3-3 4-4 5-5 6-6 7-7 8-8 9-9 10-10"), + paragraph_behavior=UpdateUsfmMarkerBehavior.PRESERVE, + style_behavior=UpdateUsfmMarkerBehavior.STRIP, + ) + rows = [ + UpdateUsfmRow(scr_ref("MAT 1:1"), "Esta es la primera parte.", metadata={"alignment_info": align_info}), + UpdateUsfmRow(scr_ref("MAT 1:2"), "Esta es la segunda parte.", metadata={"alignment_info": align_info}), + ] + usfm = r"""\id MAT +\c 1 +\v 1-2 This is the first part. +\p This is the second part. +""" + + target = update_usfm(rows, usfm, update_block_handlers=[PlaceMarkersUsfmUpdateBlockHandler()]) + result = r"""\id MAT +\c 1 +\v 1-2 Esta es la primera parte. +\p Esta es la segunda parte. +""" + assert_usfm_equals(target, result) + + def scr_ref(*refs: str) -> List[ScriptureRef]: return [ScriptureRef.parse(ref) for ref in refs] diff --git a/tests/corpora/test_update_usfm_parser_handler.py b/tests/corpora/test_update_usfm_parser_handler.py index f73aa683..33e54994 100644 --- a/tests/corpora/test_update_usfm_parser_handler.py +++ b/tests/corpora/test_update_usfm_parser_handler.py @@ -1706,6 +1706,56 @@ def test_filter_chapters_with_bad_chapter_reference() -> None: assert_usfm_equals(target, result) +def test_unclosed_style_marker_does_not_consume_next_paragraph_marker() -> None: + # An unclosed character style has no end marker of its own, so it is closed implicitly by + # the next paragraph marker. That marker belongs to the paragraph it starts, not to the + # style being closed, so it must survive. + rows = [ + UpdateUsfmRow(scr_ref("MAT 1:1"), "New verse 1"), + UpdateUsfmRow(scr_ref("MAT 1:2"), "New verse 2"), + ] + usfm = r"""\id MAT +\c 1 +\q1 +\v 1 Verse 1 \bd Selah +\b +\q1 +\v 2 Verse 2 +""" + + target = update_usfm(rows, usfm) + result = r"""\id MAT +\c 1 +\q1 +\v 1 New verse 1 +\b +\q1 +\v 2 New verse 2 +""" + assert_usfm_equals(target, result) + + # ...including when the unclosed style is in a non-verse paragraph + rows = [ + UpdateUsfmRow(scr_ref("MAT 1:0/1:d"), "New title"), + UpdateUsfmRow(scr_ref("MAT 1:1"), "New verse 1"), + ] + usfm = r"""\id MAT +\c 1 +\d \bd Title +\q1 +\v 1 Verse 1 +""" + + target = update_usfm(rows, usfm) + result = r"""\id MAT +\c 1 +\d New title +\q1 +\v 1 New verse 1 +""" + assert_usfm_equals(target, result) + + def scr_ref(*refs: str) -> List[ScriptureRef]: return [ScriptureRef.parse(ref) for ref in refs]