Code review changes requested for tests

Author: Ben King

machine/corpora/quotation_mark_update_first_pass.py

@@ -1,3 +1,4 @@
+from collections import defaultdict
 from typing import Dict, List, Set
 
 from .punctuation_analysis.chapter import Chapter
@@ -33,15 +34,12 @@ def __init__(self, source_quote_convention: QuoteConvention, target_quote_conven
     def _check_whether_fallback_mode_will_work(
         self, source_quote_convention: QuoteConvention, target_quote_convention: QuoteConvention
     ) -> bool:
-        target_marks_by_source_marks: Dict[str, Set[str]] = {}
-        for depth in range(1, source_quote_convention.num_levels + 1):
+        target_marks_by_source_marks: Dict[str, Set[str]] = defaultdict(set)
+        for depth in range(1, min(source_quote_convention.num_levels, target_quote_convention.num_levels) + 1):
             opening_quotation_mark = source_quote_convention.get_opening_quotation_mark_at_depth(depth)
-            if opening_quotation_mark not in target_marks_by_source_marks:
-                target_marks_by_source_marks[opening_quotation_mark] = set()
-            if depth <= target_quote_convention.num_levels:
-                target_marks_by_source_marks[opening_quotation_mark].add(
-                    target_quote_convention.get_closing_quotation_mark_at_depth(depth)
-                )
+            target_marks_by_source_marks[opening_quotation_mark].add(
+                target_quote_convention.get_closing_quotation_mark_at_depth(depth)
+            )
 
         for source_mark in target_marks_by_source_marks:
             if len(target_marks_by_source_marks[source_mark]) > 1:

tests/corpora/punctuation_analysis/test_depth_based_quotation_mark_resolver.py

@@ -993,19 +993,19 @@ def test_is_opening_quote() -> None:
         QuotationMarkStringMatch(TextSegment.Builder().set_text("\u201c").build(), 0, 1)
     )
     assert not standard_swedish_quotation_mark_categorizer.is_opening_quotation_mark(
-        QuotationMarkStringMatch(TextSegment.Builder().set_text("\u201d").build(), 1, 2)
+        QuotationMarkStringMatch(TextSegment.Builder().set_text("\u201d").build(), 0, 1)
     )
     assert standard_swedish_quotation_mark_categorizer.is_opening_quotation_mark(
         QuotationMarkStringMatch(TextSegment.Builder().set_text("\u201c\u201d").build(), 1, 2)
     )
     assert not standard_swedish_quotation_mark_categorizer.is_opening_quotation_mark(
-        QuotationMarkStringMatch(TextSegment.Builder().set_text("\u2019").build(), 1, 2)
+        QuotationMarkStringMatch(TextSegment.Builder().set_text("\u2019").build(), 0, 1)
     )
     assert standard_swedish_quotation_mark_categorizer.is_opening_quotation_mark(
         QuotationMarkStringMatch(TextSegment.Builder().set_text("\u201c\u2019").build(), 1, 2)
     )
     assert not three_conventions_quotation_mark_categorizer.is_opening_quotation_mark(
-        QuotationMarkStringMatch(TextSegment.Builder().set_text("\u201c").build(), 1, 2)
+        QuotationMarkStringMatch(TextSegment.Builder().set_text("\u201c").build(), 0, 1)
     )
     assert three_conventions_quotation_mark_categorizer.is_opening_quotation_mark(
         QuotationMarkStringMatch(TextSegment.Builder().set_text("\u201c\u201c").build(), 1, 2)
@@ -2290,7 +2290,6 @@ def test_too_deep_nesting_issue() -> None:
         QuotationMarkMetadata("\u2018", 2, QuotationMarkDirection.OPENING, text_segment, 6, 7),
         QuotationMarkMetadata("\u201c", 3, QuotationMarkDirection.OPENING, text_segment, 10, 11),
         QuotationMarkMetadata("\u2018", 4, QuotationMarkDirection.OPENING, text_segment, 13, 14),
-        # QuotationMarkMetadata("\u201c", 5, QuotationMarkDirection.Opening, text_segment, 20, 21),
     ]
     assert standard_english_quotation_mark_resolver.get_issues() == {
         QuotationMarkResolutionIssue.TOO_DEEP_NESTING,

tests/corpora/punctuation_analysis/test_quotation_mark_string_match.py

@@ -154,7 +154,7 @@ def test_does_trailing_substring_match() -> None:
     assert quotation_mark_string_match.trailing_substring_matches(regex.compile(r"^ text$"))
 
     quotation_mark_string_match = QuotationMarkStringMatch(
-        TextSegment.Builder().set_text("sample text").build(), 11, 12
+        TextSegment.Builder().set_text("sample text").build(), 10, 11
     )
     assert not quotation_mark_string_match.trailing_substring_matches(regex.compile(r".+"))
 

tests/corpora/punctuation_analysis/test_quote_convention.py

@@ -89,13 +89,13 @@ def test_get_num_levels() -> None:
     assert empty_quote_convention.num_levels == 0
 
     one_level_quote_convention = QuoteConvention(
-        "one-level-quote-convention",
+        "one_level_quote_convention",
         [SingleLevelQuoteConvention("\u201c", "\u201d")],
     )
     assert one_level_quote_convention.num_levels == 1
 
     two_level_quote_convention = QuoteConvention(
-        "two-level-quote-convention",
+        "two_level_quote_convention",
         [
             SingleLevelQuoteConvention("\u201c", "\u201d"),
             SingleLevelQuoteConvention("\u2018", "\u2019"),
@@ -104,7 +104,7 @@ def test_get_num_levels() -> None:
     assert two_level_quote_convention.num_levels == 2
 
     three_level_quote_convention = QuoteConvention(
-        "three-level-quote-convention",
+        "three_level_quote_convention",
         [
             SingleLevelQuoteConvention("\u201c", "\u201d"),
             SingleLevelQuoteConvention("\u2018", "\u2019"),
@@ -116,7 +116,7 @@ def test_get_num_levels() -> None:
 
 def test_get_opening_quote_at_level() -> None:
     quote_convention = QuoteConvention(
-        "test-quote-convention",
+        "test_quote_convention",
         [
             SingleLevelQuoteConvention("\u201c", "\u201d"),
             SingleLevelQuoteConvention("\u2018", "\u2019"),
@@ -130,7 +130,7 @@ def test_get_opening_quote_at_level() -> None:
 
 def test_get_closing_quote_at_level() -> None:
     quote_convention = QuoteConvention(
-        "test-quote-convention",
+        "test_quote_convention",
         [
             SingleLevelQuoteConvention("\u201c", "\u201d"),
             SingleLevelQuoteConvention("\u2018", "\u2019"),
@@ -144,7 +144,7 @@ def test_get_closing_quote_at_level() -> None:
 
 def test_get_expected_quotation_mark() -> None:
     quote_convention = QuoteConvention(
-        "test-quote-convention",
+        "test_quote_convention",
         [
             SingleLevelQuoteConvention("\u201c", "\u201d"),
             SingleLevelQuoteConvention("\u2018", "\u2019"),
@@ -168,28 +168,28 @@ def test_includes_opening_quotation_mark() -> None:
     assert not empty_quote_convention._includes_opening_quotation_mark("\u201c")
 
     positive_quote_convention1 = QuoteConvention(
-        "positive quote convention 1", [SingleLevelQuoteConvention("\u201c", "\u201d")]
+        "positive_quote_convention_1", [SingleLevelQuoteConvention("\u201c", "\u201d")]
     )
     assert positive_quote_convention1._includes_opening_quotation_mark("\u201c")
 
     negative_quote_convention1 = QuoteConvention(
-        "negative quote convention 1", [SingleLevelQuoteConvention("\u2018", "\u2019")]
+        "negative_quote_convention_1", [SingleLevelQuoteConvention("\u2018", "\u2019")]
     )
     assert not negative_quote_convention1._includes_opening_quotation_mark("\u201c")
 
     negative_quote_convention2 = QuoteConvention(
-        "negative quote convention 2", [SingleLevelQuoteConvention("\u201d", "\u201c")]
+        "negative_quote_convention_2", [SingleLevelQuoteConvention("\u201d", "\u201c")]
     )
     assert not negative_quote_convention2._includes_opening_quotation_mark("\u201c")
 
     positive_quote_convention2 = QuoteConvention(
-        "positive quote convention 2",
+        "positive_quote_convention_2",
         [SingleLevelQuoteConvention("\u201c", "\u201d"), SingleLevelQuoteConvention("\u2018", "\u2019")],
     )
     assert positive_quote_convention2._includes_opening_quotation_mark("\u201c")
 
     positive_quote_convention3 = QuoteConvention(
-        "positive quote convention 3",
+        "positive_quote_convention_3",
         [SingleLevelQuoteConvention("\u2018", "\u2019"), SingleLevelQuoteConvention("\u201c", "\u201d")],
     )
     assert positive_quote_convention3._includes_opening_quotation_mark("\u201c")
@@ -210,34 +210,34 @@ def test_includes_closing_quotation_mark() -> None:
     assert not empty_quote_convention._includes_closing_quotation_mark("\u201d")
 
     positive_quote_convention1 = QuoteConvention(
-        "positive quote convention 1", [SingleLevelQuoteConvention("\u201c", "\u201d")]
+        "positive_quote_convention_1", [SingleLevelQuoteConvention("\u201c", "\u201d")]
     )
     assert positive_quote_convention1._includes_closing_quotation_mark("\u201d")
 
     negative_quote_convention1 = QuoteConvention(
-        "negative quote convention 1", [SingleLevelQuoteConvention("\u2018", "\u2019")]
+        "negative_quote_convention_1", [SingleLevelQuoteConvention("\u2018", "\u2019")]
     )
     assert not negative_quote_convention1._includes_closing_quotation_mark("\u201d")
 
     negative_quote_convention2 = QuoteConvention(
-        "negative quote convention 2", [SingleLevelQuoteConvention("\u201d", "\u201c")]
+        "negative_quote_convention_2", [SingleLevelQuoteConvention("\u201d", "\u201c")]
     )
     assert not negative_quote_convention2._includes_closing_quotation_mark("\u201d")
 
     positive_quote_convention2 = QuoteConvention(
-        "positive quote convention 2",
+        "positive_quote_convention_2",
         [SingleLevelQuoteConvention("\u201c", "\u201d"), SingleLevelQuoteConvention("\u2018", "\u2019")],
     )
     assert positive_quote_convention2._includes_closing_quotation_mark("\u201d")
 
     positive_quote_convention3 = QuoteConvention(
-        "positive quote convention 3",
+        "positive_quote_convention_3",
         [SingleLevelQuoteConvention("\u2018", "\u2019"), SingleLevelQuoteConvention("\u201c", "\u201d")],
     )
     assert positive_quote_convention3._includes_closing_quotation_mark("\u201d")
 
     negative_quote_convention3 = QuoteConvention(
-        "negative quote convention 3",
+        "negative_quote_convention_3",
         [
             SingleLevelQuoteConvention("\u2018", "\u2019"),
             SingleLevelQuoteConvention("'", "'"),
@@ -249,7 +249,7 @@ def test_includes_closing_quotation_mark() -> None:
 
 def test_get_possible_depths() -> None:
     quote_convention = QuoteConvention(
-        "test-quote-convention",
+        "test_quote_convention",
         [
             SingleLevelQuoteConvention("\u201c", "\u201d"),
             SingleLevelQuoteConvention("\u2018", "\u2019"),
@@ -271,7 +271,7 @@ def test_get_possible_depths() -> None:
 
 def test_is_compatible_with_observed_quotation_marks() -> None:
     quote_convention = QuoteConvention(
-        "test-quote-convention",
+        "test_quote_convention",
         [
             SingleLevelQuoteConvention("\u201c", "\u201d"),
             SingleLevelQuoteConvention("\u2018", "\u2019"),
@@ -302,7 +302,7 @@ def test_normalize() -> None:
     assert normalized_empty_quote_convention.num_levels == 0
 
     standard_english_quote_convention = QuoteConvention(
-        "standard-english-quote-convention",
+        "standard_english_quote_convention",
         [
             SingleLevelQuoteConvention("\u201c", "\u201d"),
             SingleLevelQuoteConvention("\u2018", "\u2019"),
@@ -311,7 +311,7 @@ def test_normalize() -> None:
         ],
     )
     normalized_standard_english_quote_convention = standard_english_quote_convention.normalize()
-    assert normalized_standard_english_quote_convention.name == "standard-english-quote-convention_normalized"
+    assert normalized_standard_english_quote_convention.name == "standard_english_quote_convention_normalized"
     assert normalized_standard_english_quote_convention.num_levels == 4
     assert normalized_standard_english_quote_convention.get_opening_quotation_mark_at_depth(1) == '"'
     assert normalized_standard_english_quote_convention.get_closing_quotation_mark_at_depth(1) == '"'
@@ -323,15 +323,15 @@ def test_normalize() -> None:
     assert normalized_standard_english_quote_convention.get_closing_quotation_mark_at_depth(4) == "'"
 
     western_european_quote_convention = QuoteConvention(
-        "test-quote-convention",
+        "test_quote_convention",
         [
             SingleLevelQuoteConvention("\u201c", "\u201d"),
             SingleLevelQuoteConvention("\u00ab", "\u00bb"),
             SingleLevelQuoteConvention("\u2018", "\u2019"),
         ],
     )
     normalized_western_european_quote_convention = western_european_quote_convention.normalize()
-    assert normalized_western_european_quote_convention.name == "test-quote-convention_normalized"
+    assert normalized_western_european_quote_convention.name == "test_quote_convention_normalized"
     assert normalized_western_european_quote_convention.num_levels == 3
     assert normalized_western_european_quote_convention.get_opening_quotation_mark_at_depth(1) == '"'
     assert normalized_western_european_quote_convention.get_closing_quotation_mark_at_depth(1) == '"'
@@ -341,7 +341,7 @@ def test_normalize() -> None:
     assert normalized_western_european_quote_convention.get_closing_quotation_mark_at_depth(3) == "'"
 
     hybrid_british_typewriter_english_quote_convention = QuoteConvention(
-        "hybrid-british-typewriter-english-quote-convention",
+        "hybrid_british_typewriter_english_quote_convention",
         [
             SingleLevelQuoteConvention("\u00ab", "\u00bb"),
             SingleLevelQuoteConvention("'", "'"),
@@ -354,7 +354,7 @@ def test_normalize() -> None:
     )
     assert (
         normalized_hybrid_british_typewriter_english_quote_convention.name
-        == "hybrid-british-typewriter-english-quote-convention_normalized"
+        == "hybrid_british_typewriter_english_quote_convention_normalized"
     )
     assert normalized_hybrid_british_typewriter_english_quote_convention.num_levels == 3
     assert normalized_hybrid_british_typewriter_english_quote_convention.get_opening_quotation_mark_at_depth(1) == '"'
@@ -367,15 +367,15 @@ def test_normalize() -> None:
 
 def test_print_summary() -> None:
     quote_convention = QuoteConvention(
-        "test-quote-convention",
+        "test_quote_convention",
         [
             SingleLevelQuoteConvention("\u201c", "\u201D"),
             SingleLevelQuoteConvention("\u2018", "\u2019"),
             SingleLevelQuoteConvention("\u201D", "\u201D"),
         ],
     )
     expected_summary_message = (
-        "test-quote-convention\n"
+        "test_quote_convention\n"
         + "\u201CFirst-level quote\u201D\n"
         + "\u2018Second-level quote\u2019\n"
         + "\u201DThird-level quote\u201D\n"