From ada3299713d2d024a6f02c382bd68ad58e18ce4a Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Tue, 19 Oct 2021 11:31:56 -0400 Subject: [PATCH 1/5] fix wrapper on opened files --- aicsimageio/readers/bioformats_reader.py | 9 +++++++-- aicsimageio/tests/readers/test_bioformats_reader.py | 12 +++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/aicsimageio/readers/bioformats_reader.py b/aicsimageio/readers/bioformats_reader.py index 1a2bce3e1..64b408ad6 100644 --- a/aicsimageio/readers/bioformats_reader.py +++ b/aicsimageio/readers/bioformats_reader.py @@ -165,7 +165,8 @@ def physical_pixel_sizes(self) -> types.PhysicalPixelSizes: def _to_xarray(self, delayed: bool = True) -> xr.DataArray: with BioFile(self._path, **self._bf_kwargs) as rdr: # type: ignore - image_data = rdr.to_dask() if delayed else rdr.to_numpy() + f = rdr.to_dask if delayed else rdr.to_numpy + image_data = f(self.current_scene_index) _, coords = metadata_utils.get_dims_and_coords_from_ome( ome=rdr.ome_metadata, scene_index=self.current_scene_index, @@ -295,6 +296,7 @@ def __init__( mo.set(name, str(value)) self._r.setMetadataOptions(mo) + self._close_on_exit = True self.open() self._lock = Lock() self.set_series(series) @@ -403,12 +405,15 @@ def ome_metadata(self) -> OME: return OME.from_xml(xml) def __enter__(self) -> BioFile: + # entering an already opened file should not close the file on exit. + self._close_on_exit = not self.is_open if not self.is_open: self.open() return self def __exit__(self, *args: Any) -> None: - self.close() + if self._close_on_exit: + self.close() def __del__(self) -> None: self.close() diff --git a/aicsimageio/tests/readers/test_bioformats_reader.py b/aicsimageio/tests/readers/test_bioformats_reader.py index fa65117dd..7fcdfc2e4 100644 --- a/aicsimageio/tests/readers/test_bioformats_reader.py +++ b/aicsimageio/tests/readers/test_bioformats_reader.py @@ -8,7 +8,7 @@ from ome_types import OME from aicsimageio import dimensions, exceptions -from aicsimageio.readers.bioformats_reader import BioformatsReader +from aicsimageio.readers.bioformats_reader import BioFile, BioformatsReader from aicsimageio.tests.image_container_test_utils import ( run_image_file_checks, run_multi_scene_image_read_checks, @@ -404,3 +404,13 @@ def test_multi_scene_bioformats_reader( second_scene_shape=second_scene_shape, second_scene_dtype=np.dtype(np.uint16), ) + + +def test_biofile_scene_change() -> None: + """Make sure that DaskArrayProxy doesn't close an opened file.""" + uri = get_resource_full_path("ND2_dims_p4z5t3c2y32x32.nd2", LOCAL) + f = BioFile(uri) + assert isinstance(f.to_dask().compute(), np.ndarray) + f.set_series(1) + assert isinstance(f.to_dask().compute(), np.ndarray) + f.close() From c7a401fd83c4edad8b79a5e65987301e980e17a1 Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Tue, 19 Oct 2021 11:35:57 -0400 Subject: [PATCH 2/5] undo other PR change --- aicsimageio/readers/bioformats_reader.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/aicsimageio/readers/bioformats_reader.py b/aicsimageio/readers/bioformats_reader.py index 64b408ad6..96c671d52 100644 --- a/aicsimageio/readers/bioformats_reader.py +++ b/aicsimageio/readers/bioformats_reader.py @@ -165,8 +165,7 @@ def physical_pixel_sizes(self) -> types.PhysicalPixelSizes: def _to_xarray(self, delayed: bool = True) -> xr.DataArray: with BioFile(self._path, **self._bf_kwargs) as rdr: # type: ignore - f = rdr.to_dask if delayed else rdr.to_numpy - image_data = f(self.current_scene_index) + image_data = rdr.to_dask() if delayed else rdr.to_numpy() _, coords = metadata_utils.get_dims_and_coords_from_ome( ome=rdr.ome_metadata, scene_index=self.current_scene_index, From 05b9fb110920150c300797f3cce7894fddb6442a Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Wed, 10 Nov 2021 19:51:43 -0500 Subject: [PATCH 3/5] change approach --- aicsimageio/readers/bioformats_reader.py | 14 ++++----- aicsimageio/utils/dask_proxy.py | 36 +++++++++++++++--------- 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/aicsimageio/readers/bioformats_reader.py b/aicsimageio/readers/bioformats_reader.py index 96c671d52..67fde3487 100644 --- a/aicsimageio/readers/bioformats_reader.py +++ b/aicsimageio/readers/bioformats_reader.py @@ -295,7 +295,6 @@ def __init__( mo.set(name, str(value)) self._r.setMetadataOptions(mo) - self._close_on_exit = True self.open() self._lock = Lock() self.set_series(series) @@ -379,9 +378,9 @@ def to_dask(self, series: Optional[int] = None) -> DaskArrayProxy: return DaskArrayProxy(arr, self) @property - def is_open(self) -> bool: + def closed(self) -> bool: """Whether the underlying file is currently open""" - return bool(self._r.getCurrentFile()) + return not bool(self._r.getCurrentFile()) @property def filename(self) -> str: @@ -405,14 +404,11 @@ def ome_metadata(self) -> OME: def __enter__(self) -> BioFile: # entering an already opened file should not close the file on exit. - self._close_on_exit = not self.is_open - if not self.is_open: - self.open() + self.open() return self def __exit__(self, *args: Any) -> None: - if self._close_on_exit: - self.close() + self.close() def __del__(self) -> None: self.close() @@ -446,7 +442,7 @@ def _get_plane( array of requested bytes. """ with self._lock: - was_open = self.is_open + was_open = not self.closed if not was_open: self.open() diff --git a/aicsimageio/utils/dask_proxy.py b/aicsimageio/utils/dask_proxy.py index e42b7d4b8..0b7bb65ba 100644 --- a/aicsimageio/utils/dask_proxy.py +++ b/aicsimageio/utils/dask_proxy.py @@ -3,12 +3,24 @@ """ from __future__ import annotations -from typing import Any, Callable, ContextManager +from contextlib import nullcontext +from typing import TYPE_CHECKING, Any, Callable import dask.array as da import numpy as np from wrapt import ObjectProxy +if TYPE_CHECKING: + from typing_extensions import Protocol + + # fmt: off + class CheckableContext(Protocol): + @property + def closed(self) -> bool: ... # noqa: E704 + def __enter__(self) -> CheckableContext: ... # noqa: E704 + def __exit__(self, *a: Any) -> None: ... # noqa: E704 + # fmt: on + class DaskArrayProxy(ObjectProxy): """Dask array wrapper that provides a 'file open' context when computing. @@ -34,28 +46,26 @@ class DaskArrayProxy(ObjectProxy): __wrapped__: da.Array - def __init__(self, wrapped: da.Array, file_ctx: ContextManager) -> None: + def __init__(self, wrapped: da.Array, file_ctx: CheckableContext) -> None: super().__init__(wrapped) - self.__wrapped__._ctx_ = file_ctx + self._file_ctx = file_ctx def __getitem__(self, key: Any) -> DaskArrayProxy: - return DaskArrayProxy(self.__wrapped__.__getitem__(key), self.__wrapped__._ctx_) + return DaskArrayProxy(self.__wrapped__.__getitem__(key), self._file_ctx) def __getattr__(self, key: Any) -> Any: attr = getattr(self.__wrapped__, key) - return ( - _ArrayMethodProxy(attr, self.__wrapped__._ctx_) if callable(attr) else attr - ) + return _ArrayMethodProxy(attr, self._file_ctx) if callable(attr) else attr def __repr__(self) -> str: return repr(self.__wrapped__) def compute(self, **kwargs: Any) -> np.ndarray: - with self.__wrapped__._ctx_: + with self._file_ctx if self._file_ctx.closed else nullcontext(): # type: ignore return self.__wrapped__.compute(**kwargs) def __array__(self, dtype: str = None, **kwargs: Any) -> np.ndarray: - with self.__wrapped__._ctx_: + with self._file_ctx if self._file_ctx.closed else nullcontext(): # type: ignore return self.__wrapped__.__array__(dtype, **kwargs) @@ -63,17 +73,17 @@ class _ArrayMethodProxy: """Wraps method on a dask array and returns a DaskArrayProxy if the result of the method is a dask array. see details in DaskArrayProxy docstring.""" - def __init__(self, method: Callable, file_ctx: ContextManager) -> None: + def __init__(self, method: Callable, file_ctx: CheckableContext) -> None: self.method = method - self.file_ctx = file_ctx + self._file_ctx = file_ctx def __repr__(self) -> str: return repr(self.method) def __call__(self, *args: Any, **kwds: Any) -> Any: - with self.file_ctx: + with self._file_ctx if self._file_ctx.closed else nullcontext(): # type: ignore result = self.method(*args, **kwds) if isinstance(result, da.Array): - return DaskArrayProxy(result, self.file_ctx) + return DaskArrayProxy(result, self._file_ctx) return result From 7bdd2342079c960dab7ebf67c6d3e1c4f2e8018c Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Wed, 10 Nov 2021 19:52:57 -0500 Subject: [PATCH 4/5] test --- aicsimageio/tests/utils/test_dask_proxy.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/aicsimageio/tests/utils/test_dask_proxy.py b/aicsimageio/tests/utils/test_dask_proxy.py index df3686938..22fc7d7bc 100644 --- a/aicsimageio/tests/utils/test_dask_proxy.py +++ b/aicsimageio/tests/utils/test_dask_proxy.py @@ -13,6 +13,10 @@ class FileContext: FILE_OPEN = False OPEN_COUNT = 0 + @property + def closed(self) -> bool: + return not self.FILE_OPEN + def __enter__(self) -> "FileContext": if not self.FILE_OPEN: self.OPEN_COUNT += 1 From 103fc9669fa7879fab957b66dbe4f39dca3da1da Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Wed, 10 Nov 2021 20:26:43 -0500 Subject: [PATCH 5/5] remove old comment --- aicsimageio/readers/bioformats_reader.py | 1 - 1 file changed, 1 deletion(-) diff --git a/aicsimageio/readers/bioformats_reader.py b/aicsimageio/readers/bioformats_reader.py index 824a5f971..d0f72f863 100644 --- a/aicsimageio/readers/bioformats_reader.py +++ b/aicsimageio/readers/bioformats_reader.py @@ -437,7 +437,6 @@ def ome_metadata(self) -> OME: return OME.from_xml(xml) def __enter__(self) -> BioFile: - # entering an already opened file should not close the file on exit. self.open() return self