Repository: cogent3/cogent3
Pull request #1310 (2023-03-07 06:19:45)

TST: added tests of SeqView to address coverage gaps
TST:
- added `test_subsequent_slice_neg_step` to ensure all conditions with negative steps are being tested
- added `test_seqview_step_0` to test that a slice with a step of 1 throws a `ValueError`
- added `test_seqview_repr` to test the representation of a SeqView object 

BUG: 
- removed untested condition that was both unnecessary and incorrect!

coveralls: ## Pull Request Test Coverage Report for [Build 4351275220](https://coveralls.io/builds/57600404)

* **0** of **0**   changed or added relevant lines in **0** files are covered.
* **14** unchanged lines in **1** file lost coverage.
* Overall coverage decreased (**-0.004%**) to **91.329%**

---


|  Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [.nox/test-3-10/Lib/site-packages/cogent3/core/sequence.py](https://coveralls.io/builds/57600404/source?filename=.nox%2Ftest-3-10%2FLib%2Fsite-packages%2Fcogent3%2Fcore%2Fsequence.py#L1810) | 14 | 94.7% |
<!-- | **Total:** | **14** |  | -->

|  Totals | [![Coverage Status](https://coveralls.io/builds/57600404/badge)](https://coveralls.io/builds/57600404) |
| :-- | --: |
| Change from base [Build 4350820336](https://coveralls.io/builds/57598564): |  -0.004% |
| Covered Lines: | 178718 |
| Relevant Lines: | 195686 |

---
##### 💛  - [Coveralls](https://coveralls.io)


Patch:
diff --git a/src/cogent3/core/sequence.py b/src/cogent3/core/sequence.py
index 9fdb46e99..0e2d1b691 100644
--- a/src/cogent3/core/sequence.py
+++ b/src/cogent3/core/sequence.py
@@ -1548,9 +1548,7 @@ class SeqView:
         else:
             start = self.start + len(self) * self.step + slice_start * self.step
 
-        if slice_stop > len(self.seq):
-            return _zero_slice
-        elif slice_stop >= 0:
+        if slice_stop >= 0:
             stop = self.start + slice_stop * self.step
         else:  # slice_stop < 0
             stop = self.start + len(self) * self.step + slice_stop * self.step


Test patch:
diff --git a/tests/test_core/test_sequence.py b/tests/test_core/test_sequence.py
index fd8ad297f..4ac1c545f 100644
--- a/tests/test_core/test_sequence.py
+++ b/tests/test_core/test_sequence.py
@@ -1414,6 +1414,15 @@ def test_seqview_index_null():
         _ = sv[0]
 
 
+def test_seqview_step_0():
+    "Initialising or slicing a SeqView with a step of 0 should return an IndexError"
+    sv = SeqView("0123456789")
+    with pytest.raises(ValueError):
+        _ = sv[::0]
+    with pytest.raises(ValueError):
+        _ = SeqView("0123456789", step=0)
+
+
 @pytest.mark.parametrize("start", (0, 2, 4))
 def test_seqview_invalid_index(start):
     "indexing out of bounds with a forward step should raise an IndexError"
@@ -1829,6 +1838,45 @@ def test_subsequent_slice_neg_start(slice_1, slice_2):
     assert sv[slice_1][slice_2].value == seq_data[slice_1][slice_2]
 
 
+@pytest.mark.parametrize(
+    "slice_1, slice_2",
+    (
+        # WITH DEFAULTS
+        # first step -ve
+        (slice(None, None, -1), slice(None, None, None)),
+        # second step -ve
+        (slice(None, None, None), slice(None, None, -1)),
+        # both step -ve, start/stop -ve, second slice WITHIN first
+        (slice(-1, -11, -2), slice(-1, -5, -3)),
+        # both step -ve, start/stop -ve, second slice OUTSIDE first
+        (slice(-1, -11, -2), slice(-1, -11, -3)),
+        # both step -ve, start/stop +ve, second slice WITHIN first
+        (slice(10, 0, -2), slice(5, 0, -3)),
+        # both step -ve, start/stop +ve, second slice OUTSIDE first
+        (slice(10, 0, -2), slice(10, 0, -3)),
+        # first step -ve, second step +ve, second slice WITHIN first
+        (slice(10, 0, -2), slice(1, 5, 2)),
+        # first step -ve, second step +ve, second slice OUTSIDE first
+        (slice(10, 0, -2), slice(0, 10, 2)),
+        # first step +ve, second step -ve, second slice WITHIN first
+        (slice(0, 10, 2), slice(4, 0, -2)),
+        # first step +ve, second step -ve, second slice OUTSIDE first
+        (slice(0, 10, 3), slice(10, 0, -2)),
+        # first step -ve, second step +ve, second start/stop +ve
+        (slice(10, 1, -1), slice(-8, 11, 2)),
+        # first step -ve, second step +ve, second start/stop +ve
+        (slice(10, 1, -1), slice(-19, 0, -2)),
+    ),
+)
+def test_subsequent_slice_neg_step(slice_1, slice_2):
+    """SeqView should handle subsequence slices with negative step values,
+    subsequent slices may overlap or be within previous slices
+    """
+    seq_data = "0123456789"
+    sv = SeqView(seq_data)
+    assert sv[slice_1][slice_2].value == seq_data[slice_1][slice_2]
+
+
 @pytest.mark.parametrize(
     "sub_slices_triple",
     (
@@ -1848,10 +1896,10 @@ def test_subslice_3(sub_slices_triple):
 
 @pytest.mark.parametrize("start", (0, 2, -1))
 @pytest.mark.parametrize("stop", (7, 10, -11))
-@pytest.mark.parametrize("step", (1, 2, -2))
+@pytest.mark.parametrize("step", (1, -2))
 @pytest.mark.parametrize("start_2", (0, 2, -8))
 @pytest.mark.parametrize("stop_2", (2, 4))
-@pytest.mark.parametrize("step_2", (1, -2))
+@pytest.mark.parametrize("step_2", (2, -1))
 @pytest.mark.parametrize("start_3", (0, 1, -6))
 @pytest.mark.parametrize("stop_3", (4, 10, -10))
 @pytest.mark.parametrize("step_3", (2, -2))
@@ -1890,6 +1938,26 @@ def test_seqview_remove_gaps():
     assert replaced.stop == len(seq_data.replace("-", ""))
 
 
+def test_seqview_repr():
+    # case 1: Short sequence, defaults
+    seq = "ACGT"
+    view = SeqView(seq)
+    expected = "SeqView(seq='ACGT', start=0, stop=4, step=1)"
+    assert repr(view) == expected
+
+    # case 2: Long sequence
+    seq = "ACGT" * 10
+    view = SeqView(seq)
+    expected = "SeqView(seq='ACGTACGTAC...TACGT', start=0, stop=40, step=1)"
+    assert repr(view) == expected
+
+    # case 3: Non-zero start, stop, and step values
+    seq = "ACGT" * 10
+    view = SeqView(seq, start=5, stop=35, step=2)
+    expected = "SeqView(seq='ACGTACGTAC...TACGT', start=5, stop=35, step=2)"
+    assert repr(view) == expected
+
+
 # run if called from command-line
 if __name__ == "__main__":
     main()