Small changes to simplify build_primer_pairs(). (#89)

prymer/api/picking.py

@@ -141,13 +141,14 @@ def build_primer_pairs(
         # generate all the primer pairs that don't violate hard size and Tm constraints
         for lp in left_primers:
             for rp in right_primers:
-                if rp.span.end - lp.span.start + 1 > amplicon_sizes.max:
+                amp_span = PrimerPair.calculate_amplicon_span(lp, rp)
+
+                if amp_span.length > amplicon_sizes.max:
                     continue
 
-                amp_mapping = Span(refname=target.refname, start=lp.span.start, end=rp.span.end)
-                amp_bases = bases[
-                    amp_mapping.start - region_start : amp_mapping.end - region_start + 1
-                ]
+                # Since the amplicon span and the region_start are both 1-based, the minuend
+                # becomes a zero-based offset
+                amp_bases = bases[amp_span.start - region_start : amp_span.end - region_start + 1]
                 amp_tm = calculate_long_seq_tm(amp_bases)
 
                 if amp_tm < amplicon_tms.min or amp_tm > amplicon_tms.max:
@@ -160,7 +161,7 @@ def build_primer_pairs(
                 penalty = score(
                     left_primer=lp,
                     right_primer=rp,
-                    amplicon=amp_mapping,
+                    amplicon=amp_span,
                     amplicon_tm=amp_tm,
                     amplicon_sizes=amplicon_sizes,
                     amplicon_tms=amplicon_tms,

prymer/api/primer_pair.py

@@ -84,7 +84,11 @@ class PrimerPair(OligoLike):
     def __post_init__(self) -> None:
         # Derive the amplicon from the left and right primers. This must be done before
         # calling super() as `PrimerLike.id` depends on the amplicon being set
-        object.__setattr__(self, "_amplicon", self._calculate_amplicon())
+        object.__setattr__(
+            self,
+            "_amplicon",
+            PrimerPair.calculate_amplicon_span(self.left_primer, self.right_primer),
+        )
         super(PrimerPair, self).__post_init__()
 
     @property
@@ -226,29 +230,49 @@ def __str__(self) -> str:
             + f"{self.amplicon_tm}\t{self.penalty}"
         )
 
-    def _calculate_amplicon(self) -> Span:
-        """
-        Calculates the amplicon from the left and right primers, spanning from the start of the
-        left primer to the end of the right primer.
+    @staticmethod
+    def calculate_amplicon_span(left_primer: Oligo, right_primer: Oligo) -> Span:
         """
+        Calculates the amplicon Span from the left and right primers.
+
+        Args:
+            left_primer: the left primer for the amplicon
+            right_primer: the right primer for the amplicon
 
+        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 self.left_primer.span.refname != self.right_primer.span.refname:
+        if left_primer.span.refname != right_primer.span.refname:
+            raise ValueError(
+                "Left and right primers are on different references. "
+                f"Left primer ref: {left_primer.span.refname}. "
+                f"Right primer ref: {right_primer.span.refname}"
+            )
+
+        # Require that the left primer starts before the right primer
+        if left_primer.span.start > right_primer.span.start:
             raise ValueError(
-                "The reference must be the same across primers in a pair; received "
-                f"left primer ref: {self.left_primer.span.refname}, "
-                f"right primer ref: {self.right_primer.span.refname}"
+                "Left primer does not start before the right primer. "
+                f"Left primer span: {left_primer.span}, "
+                f"Right primer span: {right_primer.span}"
             )
 
-        # Require that the left primer does not start to the right of the right primer
-        if self.left_primer.span.start > self.right_primer.span.end:
+        # Require that the left primer starts before the right primer
+        if right_primer.span.end < left_primer.span.end:
             raise ValueError(
-                "Left primer start must be less than or equal to right primer end; received "
-                "left primer genome span: {self.left_primer.span}, "
-                "right primer genome span: {self.right_primer.span}"
+                "Right primer ends before left primer ends. "
+                f"Left primer span: {left_primer.span}, "
+                f"Right primer span: {right_primer.span}"
             )
 
-        return replace(self.left_primer.span, end=self.right_primer.span.end)
+        return Span(left_primer.span.refname, left_primer.span.start, right_primer.span.end)
 
     @staticmethod
     def compare(

tests/api/test_primer_pair.py

@@ -427,7 +427,7 @@ def test_reference_mismatch() -> None:
 
     pp = PRIMER_PAIR_TEST_CASES[0].primer_pair
 
-    with pytest.raises(ValueError, match="The reference must be the same across primers in a pair"):
+    with pytest.raises(ValueError, match="different references"):
         replace(
             pp,
             left_primer=replace(
@@ -436,7 +436,7 @@ def test_reference_mismatch() -> None:
             ),
         )
 
-    with pytest.raises(ValueError, match="The reference must be the same across primers in a pair"):
+    with pytest.raises(ValueError, match="different references"):
         replace(
             pp,
             right_primer=replace(
@@ -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 start must be less than or equal to right primer end"
-    ):
+    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)