lmdu/pyfastx

ValueError in test suite with pyfaidx 0.7.1

emollier opened this issue · 1 comments

Hi,

While working on packaging pyfastx for Debian, in order to fulfill the new dependency requirements of augur, I noticed that the way pyfaidx is called to produce the reference object at the negative index is somehow broken (at least with pyfaidx 0.7.1, python3.10 and python3.11 in Debian sid). It raises a -probably undue- test error:

ERROR: test_seq_by_index (tests.test_sequence.SequenceTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/build/pyfastx-LEa3AU/pyfastx-0.8.4/tests/test_sequence.py", line 54, in test_seq_by_index
    expect = self.faidx[idx][:]
  File "/usr/lib/python3/dist-packages/pyfaidx/__init__.py", line 1099, in __getitem__
    rname = next(islice(self.records.keys(), rname, None))
ValueError: Indices for islice() must be None or an integer: 0 <= x <= sys.maxsize.

Manually implementing the negative index wrap around works around the problem and allows me to proceed to the actual negative index test on pyfastx, which in turn works as expected.

--- a/tests/test_sequence.py
+++ b/tests/test_sequence.py
@@ -51,7 +51,7 @@ class SequenceTest(unittest.TestCase):
 
 		#test negative index
 		idx = (self.get_random_index() + 1) * -1
-		expect = self.faidx[idx][:]
+		expect = self.faidx[self.count+idx][:]
 		result = self.fastx[idx]
 
 		self.assertEqual(expect.name, result.name)

I'm not entirely sure the problem lies in pyfastx, or if this is the new behavior of pyfaidx which is buggy. Thus, I haven't prepared a full fledged merge request, but I thought you might be interested in being aware of the problem anyway.

Have a nice day, :)
Étienne.

lmdu commented

I recommended you use pyfastx 0.8.4. For Python 3.10 and 3.11, we are developing next version for well support Python 3.10 and 3.11