Skip to content

Commit

Permalink
Fix Bio.bgzf file mode check for BytesIO (#4270)
Browse files Browse the repository at this point in the history
* Simplify detection of open files being in binary mode by reading 0 bytes and checking that the result is b"" in BgzfReader and BgzfWriter.
* Removed an unused variable in BgzfWriter._write_block
* Added tests passing binary and non-binary fileobj arguments to bgzf.BgzfReader and bgzf.BgzfWriter
  • Loading branch information
terrycojones committed Apr 6, 2023
1 parent ca64586 commit c4a47ff
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 4 deletions.
9 changes: 5 additions & 4 deletions Bio/bgzf.py
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ def _load_bgzf_block(handle, text_mode=False):
class BgzfReader:
r"""BGZF reader, acts like a read only handle but seek/tell differ.
Let's use the BgzfBlocks function to have a peak at the BGZF blocks
Let's use the BgzfBlocks function to have a peek at the BGZF blocks
in an example BAM file,
>>> from builtins import open
Expand Down Expand Up @@ -597,8 +597,9 @@ def __init__(self, filename=None, mode="r", fileobj=None, max_cache=100):
raise ValueError(
"Must use a read mode like 'r' (default), 'rt', or 'rb' for binary"
)
# If an open file was passed, make sure it was opened in binary mode.
if fileobj:
if "b" not in fileobj.mode.lower():
if fileobj.read(0) != b"":
raise ValueError("fileobj not opened in binary mode")
handle = fileobj
else:
Expand Down Expand Up @@ -798,8 +799,9 @@ def __init__(self, filename=None, mode="w", fileobj=None, compresslevel=6):
"""Initilize the class."""
if filename and fileobj:
raise ValueError("Supply either filename or fileobj, not both")
# If an open file was passed, make sure it was opened in binary mode.
if fileobj:
if "b" not in fileobj.mode.lower():
if fileobj.read(0) != b"":
raise ValueError("fileobj not opened in binary mode")
handle = fileobj
else:
Expand All @@ -817,7 +819,6 @@ def __init__(self, filename=None, mode="w", fileobj=None, compresslevel=6):
def _write_block(self, block):
"""Write provided data to file as a single BGZF compressed block (PRIVATE)."""
# print("Saving %i bytes" % len(block))
start_offset = self._handle.tell()
if len(block) > 65536:
raise ValueError(f"{len(block)} Block length > 65536")
# Giving a negative window bits means no gzip/zlib headers,
Expand Down
23 changes: 23 additions & 0 deletions Tests/test_bgzf.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import os
import tempfile
from random import shuffle
import io

from Bio import bgzf

Expand Down Expand Up @@ -465,6 +466,28 @@ def test_BgzfBlocks_TypeError(self):
with self.assertRaises(TypeError):
list(bgzf.BgzfBlocks(decompressed))

def test_reader_with_binary_fileobj(self):
"""A BgzfReader must accept a binary mode file object."""
reader = bgzf.BgzfReader(fileobj=io.BytesIO())
self.assertEqual(0, reader.tell())

def test_reader_with_non_binary_fileobj(self):
"""A BgzfReader must raise ValueError on a non-binary file object."""
error = "^fileobj not opened in binary mode$"
with self.assertRaisesRegex(ValueError, error):
bgzf.BgzfReader(fileobj=io.StringIO())

def test_writer_with_binary_fileobj(self):
"""A BgzfWriter must accept a binary mode file object."""
writer = bgzf.BgzfWriter(fileobj=io.BytesIO())
self.assertEqual(0, writer.tell())

def test_writer_with_non_binary_fileobj(self):
"""A BgzfWriter must raise ValueError on a non-binary file object."""
error = "^fileobj not opened in binary mode$"
with self.assertRaisesRegex(ValueError, error):
bgzf.BgzfWriter(fileobj=io.StringIO())


if __name__ == "__main__":
runner = unittest.TextTestRunner(verbosity=2)
Expand Down

0 comments on commit c4a47ff

Please sign in to comment.