From eaf712cba33473e29ad2c0c2dec2e83ef417b532 Mon Sep 17 00:00:00 2001 From: Marcel Martin Date: Sun, 12 Nov 2023 15:14:30 +0100 Subject: [PATCH 1/3] Let PipedCompressionWriter/-Reader derive from IOBase This is the proper thing to do and also gives us a couple of methods for free, in particular readlines(). This also allows us to get rid of the Closing mixin. Closes #129 --- src/xopen/__init__.py | 38 ++++++-------------------------------- tests/test_piped.py | 6 ++++++ 2 files changed, 12 insertions(+), 32 deletions(-) diff --git a/src/xopen/__init__.py b/src/xopen/__init__.py index 3d00694..afac29a 100644 --- a/src/xopen/__init__.py +++ b/src/xopen/__init__.py @@ -33,7 +33,6 @@ import subprocess import tempfile import time -from abc import ABC, abstractmethod from subprocess import Popen, PIPE, DEVNULL from typing import ( Optional, @@ -169,30 +168,7 @@ def _can_read_concatenated_gz(program: str) -> bool: os.remove(temp_path) -class Closing(ABC): - """ - Inherit from this class and implement a close() method to offer context - manager functionality. - """ - - def __enter__(self): - return self - - def __exit__(self, *exc_info): - self.close() - - def __del__(self): - try: - self.close() - except Exception: - pass - - @abstractmethod - def close(self): - """Called when exiting the context manager""" - - -class PipedCompressionWriter(Closing): +class PipedCompressionWriter(io.IOBase): """ Write Compressed files by running an external process and piping into it. """ @@ -228,7 +204,6 @@ def __init__( # TODO use a context manager self.outfile = open(path, mode[0] + "b") - self.closed: bool = False self.name: str = str(os.fspath(path)) self._mode: str = mode self._program_args: List[str] = program_args @@ -295,7 +270,7 @@ def write(self, arg: AnyStr) -> int: def close(self) -> None: if self.closed: return - self.closed = True + super().close() self._file.close() retcode = self.process.wait() self.outfile.close() @@ -323,7 +298,7 @@ def __next__(self): raise io.UnsupportedOperation("not readable") -class PipedCompressionReader(Closing): +class PipedCompressionReader(io.IOBase): """ Open a pipe to a process for reading a compressed file. """ @@ -383,7 +358,6 @@ def __init__( ) else: self._file = self.process.stdout - self.closed = False self._wait_for_output_or_process_exit() self._raise_if_error() @@ -399,7 +373,7 @@ def __repr__(self): def close(self) -> None: if self.closed: return - self.closed = True + super().close() retcode = self.process.poll() check_allowed_code_and_message = False if retcode is None: @@ -413,7 +387,7 @@ def close(self) -> None: def __iter__(self): return self - def __next__(self) -> Union[str, bytes]: + def __next__(self) -> Union[str, bytes]: # type: ignore # incompatible with type in IOBase return self._file.__next__() def _wait_for_output_or_process_exit(self): @@ -482,7 +456,7 @@ def read(self, *args) -> Union[str, bytes]: def readinto(self, *args): return self._file.readinto(*args) - def readline(self, *args) -> Union[str, bytes]: + def readline(self, *args) -> Union[str, bytes]: # type: ignore # incompatible w/type in IOBase return self._file.readline(*args) def seekable(self) -> bool: diff --git a/tests/test_piped.py b/tests/test_piped.py index 9eee7be..c70e6bf 100644 --- a/tests/test_piped.py +++ b/tests/test_piped.py @@ -175,6 +175,12 @@ def test_reader_readline_text(reader): assert f.readline() == CONTENT_LINES[0] +def test_reader_readlines(reader): + opener, extension = reader + with opener(TEST_DIR / f"file.txt{extension}", "r") as f: + assert f.readlines() == CONTENT_LINES + + @pytest.mark.parametrize("threads", [None, 1, 2]) def test_piped_reader_iter(threads, threaded_reader): opener, extension = threaded_reader From e8150f280a4bc83ebccd176cf30c41da83c5bc23 Mon Sep 17 00:00:00 2001 From: Marcel Martin Date: Sun, 14 Jan 2024 22:53:29 +0100 Subject: [PATCH 2/3] Close files properly in tests --- tests/test_piped.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/test_piped.py b/tests/test_piped.py index c70e6bf..43a5a40 100644 --- a/tests/test_piped.py +++ b/tests/test_piped.py @@ -203,15 +203,17 @@ def test_writer(tmp_path, writer): def test_writer_has_iter_method(tmp_path, writer): opener, extension = writer - with opener(tmp_path / f"out.{extension}") as f: + with opener(tmp_path / f"out{extension}") as f: f.write("hello") assert hasattr(f, "__iter__") def test_reader_iter_without_with(reader): opener, extension = reader - it = iter(opener(TEST_DIR / f"file.txt{extension}")) + f = opener(TEST_DIR / f"file.txt{extension}") + it = iter(f) assert CONTENT_LINES[0] == next(it) + f.close() @pytest.mark.parametrize("mode", ["rb", "rt"]) @@ -295,14 +297,16 @@ def test_iter_method_writers(writer, tmp_path): opener, extension = writer writer = opener(tmp_path / f"test{extension}", "wb") assert iter(writer) == writer + writer.close() def test_next_method_writers(writer, tmp_path): opener, extension = writer - writer = opener(tmp_path / f"test.{extension}", "wb") + writer = opener(tmp_path / f"test{extension}", "wb") with pytest.raises(io.UnsupportedOperation) as error: next(writer) error.match("not readable") + writer.close() def test_pipedcompressionreader_wrong_mode(): From 3fb78b21f68cdd86ed44eb8216479294d2ff3b2f Mon Sep 17 00:00:00 2001 From: Marcel Martin Date: Mon, 15 Jan 2024 00:05:41 +0100 Subject: [PATCH 3/3] Fix warnings when closing half-initialized files --- src/xopen/__init__.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/xopen/__init__.py b/src/xopen/__init__.py index afac29a..9014014 100644 --- a/src/xopen/__init__.py +++ b/src/xopen/__init__.py @@ -202,8 +202,6 @@ def __init__( ) ) - # TODO use a context manager - self.outfile = open(path, mode[0] + "b") self.name: str = str(os.fspath(path)) self._mode: str = mode self._program_args: List[str] = program_args @@ -212,6 +210,7 @@ def __init__( if threads is None: threads = min(_available_cpu_count(), 4) self._threads = threads + self.outfile = open(path, mode[0] + "b") try: self.process = self._open_process( mode, compresslevel, threads, self.outfile @@ -271,6 +270,9 @@ def close(self) -> None: if self.closed: return super().close() + if not hasattr(self, "process"): + # Exception was raised during __init__ + return self._file.close() retcode = self.process.wait() self.outfile.close() @@ -322,7 +324,7 @@ def __init__( newline=None, ): """ - Raise an OSError when pigz could not be found. + Raise an OSError when the binary could not be found. """ if mode not in ("r", "rt", "rb"): raise ValueError( @@ -345,13 +347,13 @@ def __init__( threads = 1 program_args += [f"{threads_flag}{threads}"] self._threads = threads - self.process = Popen(program_args, stdout=PIPE, stderr=PIPE) self.name = path + self._mode = mode + self.process = Popen(program_args, stdout=PIPE, stderr=PIPE) assert self.process.stdout is not None _set_pipe_size_to_max(self.process.stdout.fileno()) - self._mode = mode if "b" not in mode: self._file: IO = io.TextIOWrapper( self.process.stdout, encoding=encoding, errors=errors, newline=newline @@ -374,6 +376,9 @@ def close(self) -> None: if self.closed: return super().close() + if not hasattr(self, "process"): + # Exception was raised during __init__ + return retcode = self.process.poll() check_allowed_code_and_message = False if retcode is None: @@ -445,6 +450,8 @@ def _raise_if_error( assert self.process.stderr is not None if not stderr_message: + if self.process.stderr.closed: + return stderr_message = self.process.stderr.read() self._file.close()