Fixups from code review.

Author: tfenne

prymer/api/primer_pair.py

@@ -242,8 +242,12 @@ def calculate_amplicon_span(left_primer: Oligo, right_primer: Oligo) -> Span:
         Returns:
             a Span starting at the first base of the left primer and ending at the last base of
              the right primer
-        """
 
+        Raises:
+            ValueError: If `left_primer` and `right_primer` have different reference names.
+            ValueError: If `left_primer` doesn't start before the right primer.
+            ValueError: If `right_primer` ends before `left_primer`.
+        """
         # Require that `left_primer` and `right_primer` both map to the same reference sequence
         if left_primer.span.refname != right_primer.span.refname:
             raise ValueError(
@@ -260,6 +264,14 @@ def calculate_amplicon_span(left_primer: Oligo, right_primer: Oligo) -> Span:
                 f"Right primer span: {right_primer.span}"
             )
 
+        # Require that the left primer starts before the right primer
+        if right_primer.span.end < left_primer.span.end:
+            raise ValueError(
+                "Right primer ends before left primer ends. "
+                f"Left primer span: {left_primer.span}, "
+                f"Right primer span: {right_primer.span}"
+            )
+
         return Span(left_primer.span.refname, left_primer.span.start, right_primer.span.end)
 
     @staticmethod

tests/api/test_primer_pair.py

@@ -449,9 +449,7 @@ def test_reference_mismatch() -> None:
 def test_right_primer_before_left_primer() -> None:
     """Test that an exception is raised if the left primer starts after the right primer ends"""
     pp = PRIMER_PAIR_TEST_CASES[0].primer_pair
-    with pytest.raises(
-        ValueError, match="Left primer does not start before the right primer"
-    ):
+    with pytest.raises(ValueError, match="Left primer does not start before the right primer"):
         replace(
             pp,
             left_primer=pp.right_primer,
@@ -556,3 +554,24 @@ def test_primer_pair_compare(
     assert -expected_by_amplicon_false == PrimerPair.compare(
         this=that, that=this, seq_dict=seq_dict, by_amplicon=False
     )
+
+
+def test_calculate_amplicon_span() -> None:
+    left = Oligo(name="l", bases="AACCGGTTAA", tm=60, penalty=1, span=Span("chr1", 50, 59))
+    right = Oligo(name="l", bases="AACCGGTTAA", tm=60, penalty=1, span=Span("chr1", 150, 159))
+    assert PrimerPair.calculate_amplicon_span(left, right) == Span("chr1", 50, 159)
+
+    left = Oligo(name="l", bases="AACCGGTTAA", tm=60, penalty=1, span=Span("chr2", 50, 59))
+    right = Oligo(name="l", bases="AACCGGTTAA", tm=60, penalty=1, span=Span("chr3", 150, 159))
+    with pytest.raises(ValueError, match="different references"):
+        PrimerPair.calculate_amplicon_span(left, right)
+
+    left = Oligo(name="l", bases="AACCGGTTAA", tm=60, penalty=1, span=Span("chr1", 150, 159))
+    right = Oligo(name="l", bases="AACCGGTTAA", tm=60, penalty=1, span=Span("chr1", 50, 59))
+    with pytest.raises(ValueError, match="Left primer does not start before the right primer"):
+        PrimerPair.calculate_amplicon_span(left, right)
+
+    left = Oligo(name="l", bases="AACCGGTTAAACGTT", tm=60, penalty=1, span=Span("chr1", 150, 164))
+    right = Oligo(name="l", bases="AACCGGTTAA", tm=60, penalty=1, span=Span("chr1", 150, 159))
+    with pytest.raises(ValueError, match="Right primer ends before left primer ends"):
+        PrimerPair.calculate_amplicon_span(left, right)