From 333be85b8aba8d238d6ff7a36735e96eec5b6d52 Mon Sep 17 00:00:00 2001 From: Victor Lin <13424970+victorlin@users.noreply.github.com> Date: Mon, 3 Jul 2023 10:17:30 -0700 Subject: [PATCH 1/4] Fix docstring typing This function is written in a way that supports more than just StringIO. --- augur/io/file.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/augur/io/file.py b/augur/io/file.py index 146455081..ff26f89d2 100644 --- a/augur/io/file.py +++ b/augur/io/file.py @@ -10,7 +10,7 @@ def open_file(path_or_buffer, mode="r", **kwargs): Parameters ---------- - path_or_buffer : str or `os.PathLike` or `io.StringIO` + path_or_buffer : str or `os.PathLike` or any I/O class Name of the file to open or an existing IO buffer mode : str From 48b864609d63df4ccf296b97bb5ea1109f668dd5 Mon Sep 17 00:00:00 2001 From: Victor Lin <13424970+victorlin@users.noreply.github.com> Date: Mon, 3 Jul 2023 11:19:03 -0700 Subject: [PATCH 2/4] Add test to show existing behavior --- tests/io/test_file.py | 45 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/io/test_file.py b/tests/io/test_file.py index 387674c3b..446cde663 100644 --- a/tests/io/test_file.py +++ b/tests/io/test_file.py @@ -1,3 +1,4 @@ +import pytest import augur.io.file @@ -71,3 +72,47 @@ def test_open_file_write_zstd(self, tmpdir): f_write.write('foo\nbar\n') with zstd.open(path, 'rt') as f_read: assert f_read.read() == 'foo\nbar\n' + + def test_open_file_nested_read(self, tmpdir): + """Open a text file as a path then buffer for reading.""" + path = str(tmpdir / 'test.txt') + with open(path, 'wt') as f_write: + f_write.write('foo\nbar\n') + + with augur.io.file.open_file(path) as f_read: + with augur.io.file.open_file(f_read) as f_read: + assert f_read.read() == 'foo\nbar\n' + + def test_open_file_nested_read_zstd(self, tmpdir): + """Open a zstd-compressed text file as a path then buffer for reading.""" + import zstandard as zstd + path = str(tmpdir / 'test.txt.zst') + with zstd.open(path, 'wt') as f_write: + f_write.write('foo\nbar\n') + + with augur.io.file.open_file(path) as f_read: + with augur.io.file.open_file(f_read) as f_read: + assert f_read.read() == 'foo\nbar\n' + + def test_open_file_nested_write(self, tmpdir): + """Open a text file as a path then buffer for writing.""" + path = str(tmpdir / 'test.txt') + with augur.io.file.open_file(path, 'w') as f_write: + with augur.io.file.open_file(f_write, 'w') as f_write: + f_write.write('foo\nbar\n') + + def test_open_file_nested_write_zstd(self, tmpdir): + """Open a zstd-compressed text file as a path then buffer for writing.""" + path = str(tmpdir / 'test.txt.zst') + with augur.io.file.open_file(path, 'w') as f_write: + with augur.io.file.open_file(f_write, 'w') as f_write: + f_write.write('foo\nbar\n') + + def test_open_file_read_error(self): + """Try reading from an unsupported type.""" + path_or_buffer = len("bogus") + + # TODO: Raise a more informative error. + with pytest.raises(AttributeError, match="'int' object has no attribute 'read'"): + with augur.io.file.open_file(path_or_buffer, 'r') as f: + f.read() From 5209f690d91ed7145b3ece6771d52cc09453e7ad Mon Sep 17 00:00:00 2001 From: Victor Lin <13424970+victorlin@users.noreply.github.com> Date: Fri, 30 Jun 2023 15:34:16 -0700 Subject: [PATCH 3/4] Add run-time type restrictions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, anything that couldn't be opened with xopen would be returned as-is. The docstring described a type restriction on path_or_buffer but it was not enforced. Instead of adding a compile-time type restriction on path_or_buffer, add run-time type restrictions by using type narrowing¹ to provide clear context to static type checkers and raise a meaningful error if the type is not supported. ¹ https://mypy.readthedocs.io/en/stable/type_narrowing.html --- augur/io/file.py | 14 ++++++++++---- tests/io/test_file.py | 3 +-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/augur/io/file.py b/augur/io/file.py index ff26f89d2..1104f89cf 100644 --- a/augur/io/file.py +++ b/augur/io/file.py @@ -1,5 +1,7 @@ +import os from contextlib import contextmanager -from xopen import xopen +from io import IOBase +from xopen import PipedCompressionReader, PipedCompressionWriter, xopen @contextmanager @@ -10,7 +12,7 @@ def open_file(path_or_buffer, mode="r", **kwargs): Parameters ---------- - path_or_buffer : str or `os.PathLike` or any I/O class + path_or_buffer Name of the file to open or an existing IO buffer mode : str @@ -22,8 +24,12 @@ def open_file(path_or_buffer, mode="r", **kwargs): File handle object """ - try: + if isinstance(path_or_buffer, (str, os.PathLike)): with xopen(path_or_buffer, mode, **kwargs) as handle: yield handle - except TypeError: + + elif isinstance(path_or_buffer, (IOBase, PipedCompressionReader, PipedCompressionWriter)): yield path_or_buffer + + else: + raise TypeError(f"Type {type(path_or_buffer)} is not supported.") diff --git a/tests/io/test_file.py b/tests/io/test_file.py index 446cde663..51077ef96 100644 --- a/tests/io/test_file.py +++ b/tests/io/test_file.py @@ -112,7 +112,6 @@ def test_open_file_read_error(self): """Try reading from an unsupported type.""" path_or_buffer = len("bogus") - # TODO: Raise a more informative error. - with pytest.raises(AttributeError, match="'int' object has no attribute 'read'"): + with pytest.raises(TypeError, match="Type is not supported."): with augur.io.file.open_file(path_or_buffer, 'r') as f: f.read() From 2d02ec2bea8beaf05ab146da179ff6411b131217 Mon Sep 17 00:00:00 2001 From: Victor Lin <13424970+victorlin@users.noreply.github.com> Date: Mon, 3 Jul 2023 11:08:42 -0700 Subject: [PATCH 4/4] Update changelog --- CHANGES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 69eaf3769..e91dd5d32 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -9,9 +9,11 @@ ### Bug fixes * parse: Fix a bug where `--fix-dates` was always applied, with a default of `--fix-dates=monthfirst`. Now, running without `--fix-dates` will leave dates as-is. [#1247][] (@victorlin) +* `augur.io.open_file`: Previously, the docs described a type restriction on `path_or_buffer` but it was not enforced. It has been updated to allow all I/O classes, and is enforced at run-time. [#1250][] (@victorlin) [#1240]: https://github.com/nextstrain/augur/pull/1240 [#1247]: https://github.com/nextstrain/augur/issues/1247 +[#1250]: https://github.com/nextstrain/augur/pull/1250 ## 22.0.3 (14 June 2023)