From 00a1b9066b57037a38bc9977d5711e86d7b1dea6 Mon Sep 17 00:00:00 2001 From: Jan-Lukas Wynen Date: Mon, 9 Sep 2024 15:56:59 +0200 Subject: [PATCH 1/5] Allow passing a h5py file --- src/scippnexus/file.py | 24 +++++++++++++--- tests/file_test.py | 65 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 4 deletions(-) create mode 100644 tests/file_test.py diff --git a/src/scippnexus/file.py b/src/scippnexus/file.py index 930ec516..8572484c 100644 --- a/src/scippnexus/file.py +++ b/src/scippnexus/file.py @@ -1,6 +1,8 @@ # SPDX-License-Identifier: BSD-3-Clause -# Copyright (c) 2023 Scipp contributors (https://github.com/scipp) +# Copyright (c) 2024 Scipp contributors (https://github.com/scipp) # @author Simon Heybrock +import io +import os from contextlib import AbstractContextManager import h5py @@ -17,6 +19,7 @@ class File(AbstractContextManager, Group): def __init__( self, + name: str | os.PathLike[str] | io.BytesIO | h5py.Group, *args, definitions: Definitions | DefaultDefinitionsType = DefaultDefinitions, **kwargs, @@ -27,6 +30,9 @@ def __init__( Parameters ---------- + name: + Path, Bytes object, or h5py.Group. + Specifies the file to open. definitions: Mapping of NX_class names to application-specific definitions. The default is to use the base definitions as defined in the @@ -34,15 +40,25 @@ def __init__( """ if definitions is DefaultDefinitions: definitions = base_definitions() - self._file = h5py.File(*args, **kwargs) + + if isinstance(name, h5py.File | h5py.Group): + if args or kwargs: + raise TypeError('Cannot provide both h5py.File and other arguments') + self._file = name + self._manage_file_context = False + else: + self._file = h5py.File(name, *args, **kwargs) + self._manage_file_context = True super().__init__(self._file, definitions=definitions) def __enter__(self): - self._file.__enter__() + if self._manage_file_context: + self._file.__enter__() return self def __exit__(self, exc_type, exc_value, traceback): - self._file.close() + if self._manage_file_context: + self._file.close() def close(self): self._file.close() diff --git a/tests/file_test.py b/tests/file_test.py new file mode 100644 index 00000000..fde0b895 --- /dev/null +++ b/tests/file_test.py @@ -0,0 +1,65 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright (c) 2024 Scipp contributors (https://github.com/scipp) + +import io +from pathlib import Path + +import h5py as h5 +import pytest + +import scippnexus as snx + + +def is_closed(file: snx.File) -> bool: + try: + file.create_class('x', 'NXentry') + except ValueError as err: + return 'Unable to synchronously create group' in err.args[0] + return False + + +@pytest.mark.parametrize('path_type', [str, Path]) +def test_load_entry_from_filename(tmp_path, path_type): + with h5.File(tmp_path / 'test.nxs', 'w') as f: + snx.create_class(f, 'entry', snx.NXentry) + + with snx.File(path_type(tmp_path / 'test.nxs'), 'r') as f: + assert f.keys() == {'entry'} + assert is_closed(f) + + +def test_load_entry_from_buffer(tmp_path): + buffer = io.BytesIO() + with h5.File(buffer, 'w') as f: + snx.create_class(f, 'entry', snx.NXentry) + buffer.seek(0) + + with snx.File(buffer, 'r') as f: + assert f.keys() == {'entry'} + assert is_closed(f) + + +def test_load_entry_from_h5py_group_toor(tmp_path): + with h5.File('test.nxs', 'w', driver='core', backing_store=False) as h5_file: + snx.create_class(h5_file, 'entry', snx.NXentry) + with snx.File(h5_file) as snx_file: + assert snx_file.keys() == {'entry'} + # Not great, but we don't want to close the h5py file ourselves. + assert not is_closed(snx_file) + + +def test_load_entry_from_h5py_group_not_root(tmp_path): + with h5.File('test.nxs', 'w', driver='core', backing_store=False) as h5_file: + entry = snx.create_class(h5_file, 'entry', snx.NXentry) + snx.create_class(entry, 'user', snx.NXuser) + with snx.File(h5_file['entry']) as snx_file: + assert snx_file.keys() == {'user'} + # Not great, but we don't want to close the h5py file ourselves. + assert not is_closed(snx_file) + + +def test_file_from_h5py_group_does_not_allow_extra_args(tmp_path): + with h5.File('test.nxs', 'w', driver='core', backing_store=False) as h5_file: + snx.create_class(h5_file, 'entry', snx.NXentry) + with pytest.raises(TypeError): + snx.File(h5_file, 'r') From cb4b040bcc49336c462263799664494f51a1d138 Mon Sep 17 00:00:00 2001 From: Jan-Lukas Wynen Date: Tue, 10 Sep 2024 10:58:59 +0200 Subject: [PATCH 2/5] Fix typo Co-authored-by: Neil Vaytet <39047984+nvaytet@users.noreply.github.com> --- tests/file_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/file_test.py b/tests/file_test.py index fde0b895..8d00c988 100644 --- a/tests/file_test.py +++ b/tests/file_test.py @@ -39,7 +39,7 @@ def test_load_entry_from_buffer(tmp_path): assert is_closed(f) -def test_load_entry_from_h5py_group_toor(tmp_path): +def test_load_entry_from_h5py_group_root(tmp_path): with h5.File('test.nxs', 'w', driver='core', backing_store=False) as h5_file: snx.create_class(h5_file, 'entry', snx.NXentry) with snx.File(h5_file) as snx_file: From 2acc9e9cb024f4fb1eed0dce3a9bb28767401613 Mon Sep 17 00:00:00 2001 From: Jan-Lukas Wynen Date: Tue, 10 Sep 2024 10:59:59 +0200 Subject: [PATCH 3/5] Match error message Co-authored-by: Neil Vaytet <39047984+nvaytet@users.noreply.github.com> --- tests/file_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/file_test.py b/tests/file_test.py index 8d00c988..23c06ebf 100644 --- a/tests/file_test.py +++ b/tests/file_test.py @@ -61,5 +61,5 @@ def test_load_entry_from_h5py_group_not_root(tmp_path): def test_file_from_h5py_group_does_not_allow_extra_args(tmp_path): with h5.File('test.nxs', 'w', driver='core', backing_store=False) as h5_file: snx.create_class(h5_file, 'entry', snx.NXentry) - with pytest.raises(TypeError): + with pytest.raises(TypeError, match='Cannot provide both h5py.File and other arguments'): snx.File(h5_file, 'r') From c0e065ddd078fd940b12852e07efb5977b12561d Mon Sep 17 00:00:00 2001 From: Jan-Lukas Wynen Date: Tue, 10 Sep 2024 10:57:07 +0200 Subject: [PATCH 4/5] Explain handling of h5py.File --- src/scippnexus/file.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/scippnexus/file.py b/src/scippnexus/file.py index 8572484c..939cec50 100644 --- a/src/scippnexus/file.py +++ b/src/scippnexus/file.py @@ -31,8 +31,9 @@ def __init__( Parameters ---------- name: - Path, Bytes object, or h5py.Group. Specifies the file to open. + If this is a :class:`hyp5.File` object, the `:class:`File` will wrap + this file handle but will not close it when used as a context manager. definitions: Mapping of NX_class names to application-specific definitions. The default is to use the base definitions as defined in the From ea6b3c22966bffe0dc3b5e91f4de90e29580ba4f Mon Sep 17 00:00:00 2001 From: "pre-commit-ci-lite[bot]" <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Date: Tue, 10 Sep 2024 09:00:41 +0000 Subject: [PATCH 5/5] Apply automatic formatting --- tests/file_test.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/file_test.py b/tests/file_test.py index 23c06ebf..59e0e9d6 100644 --- a/tests/file_test.py +++ b/tests/file_test.py @@ -61,5 +61,7 @@ def test_load_entry_from_h5py_group_not_root(tmp_path): def test_file_from_h5py_group_does_not_allow_extra_args(tmp_path): with h5.File('test.nxs', 'w', driver='core', backing_store=False) as h5_file: snx.create_class(h5_file, 'entry', snx.NXentry) - with pytest.raises(TypeError, match='Cannot provide both h5py.File and other arguments'): + with pytest.raises( + TypeError, match='Cannot provide both h5py.File and other arguments' + ): snx.File(h5_file, 'r')