refactor: clarify chrom1/chrom2 logic

fulcrumgenomics · Oct 17, 2023 · af6caab · af6caab
1 parent 2baa8f0
commit af6caab
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 8 deletions.
diff --git a/fgpyo/sam/builder.py b/fgpyo/sam/builder.py
@@ -433,16 +433,16 @@ def add_pair(
             chrom1 = chrom2 = chrom
         elif chrom2 is None:
             # permit adding pair with R1 mapped and R2 unmapped, using `chrom1` syntax
-            if start2 == sam.NO_REF_POS and start1 != sam.NO_REF_POS:
-                chrom2 = sam.NO_REF_NAME
+            if start2 != sam.NO_REF_POS:
+                raise ValueError("start2 cannot be used on its own - use with chrom or chrom2.")
             else:
-                raise ValueError("When using chrom1 or chrom2, both must be specified.")
+                chrom2 = sam.NO_REF_NAME
         elif chrom1 is None:
             # permit adding pair with R2 mapped and R1 unmapped, using `chrom2` syntax
-            if start1 == sam.NO_REF_POS and start2 != sam.NO_REF_POS:
-                chrom1 = sam.NO_REF_NAME
+            if start1 != sam.NO_REF_POS:
+                raise ValueError("start1 cannot be used on its own - use with chrom or chrom1.")
             else:
-                raise ValueError("When using chrom1 or chrom2, both must be specified.")
+                chrom1 = sam.NO_REF_NAME
 
         # Setup R1
         r1 = self._new_rec(name=name, chrom=chrom1, start=start1, mapq=mapq1, attrs=attrs)

diff --git a/fgpyo/sam/tests/test_builder.py b/fgpyo/sam/tests/test_builder.py
@@ -187,10 +187,10 @@ def test_chrom1_chrom2() -> None:
     assert not r2.is_unmapped
     assert r1.is_unmapped
 
-    with pytest.raises(ValueError, match="When using chrom1 or chrom2, both must be specified."):
+    with pytest.raises(ValueError, match="start2 cannot be used on its own"):
         r1, r2 = builder.add_pair(chrom1="chr1", start1=1000, start2=1000)
 
-    with pytest.raises(ValueError, match="When using chrom1 or chrom2, both must be specified."):
+    with pytest.raises(ValueError, match="start1 cannot be used on its own"):
         r1, r2 = builder.add_pair(chrom2="chr1", start1=1000, start2=1000)