diff --git a/CHANGELOG.md b/CHANGELOG.md index bd53a91..577cecb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ ## [2.0.0] - Unreleased +### Resource-handle pass (2026-08-17) + +- Added `saxsabs.io.detector_images` as the common copy-and-close detector + loader. Shared-reference, strict 2D main/resume, integrate1d mask/EDF, and + Workbench FabIO readers now copy owned pixels/headers and close handles on + both the success and failure paths. Workbench optional `.npy` mask/flat loads + refuse pickled arrays. + ### JOSS reviewer polish (2026-08-16) - Make `saxsabs estimate-k --ref` optional so the CLI can use the same built-in @@ -107,8 +115,8 @@ alpha uncertainty, propagation model, and uncertainty type in portable provenance. Frame reports include `BufferAlphaUncertainty`. - Closed FabIO handles in the strict 1D readers and strict 2D resume verifier. - Shared-reference, strict-2D main-read, and Workbench readers still require a - common close-safe loader. + A later resource-handle pass (2026-08-17) added the common close-safe loader + for the remaining shared-reference, strict-2D main-read, and Workbench paths. ### Fixed - Deferred the TkAgg backend selection until the desktop Workbench is actually diff --git a/SASAbs.py b/SASAbs.py index 22ed9e0..91c6e8c 100644 --- a/SASAbs.py +++ b/SASAbs.py @@ -1383,6 +1383,54 @@ def _profile_uncertainty(profile): make_sample_id = None write_calibrated2d_package = None +try: + from saxsabs.io.detector_images import ( + load_detector_header as _core_load_detector_header, + load_detector_image as _core_load_detector_image, + ) +except Exception: + _core_load_detector_header = None + _core_load_detector_image = None + + +def _close_detector_handle(image): + close = getattr(image, "close", None) + if callable(close): + close() + + +def _workbench_load_detector_image(path, *, dtype=np.float64): + """Copy pixels/header through the shared loader, keeping fabio.open patchable.""" + if _core_load_detector_image is not None: + return _core_load_detector_image(path, dtype=dtype, open_image_fn=fabio.open) + image = fabio.open(path) + try: + raw = getattr(image, "data", None) + if raw is None: + raise ValueError(f"detector image has no pixel data: {path}") + if dtype is None: + data = np.array(raw, copy=True) + else: + data = np.array(raw, dtype=dtype, copy=True, order="C") + header = dict(getattr(image, "header", None) or {}) + return SimpleNamespace(data=data, header=header) + finally: + _close_detector_handle(image) + + +def _workbench_load_detector_header(path): + if _core_load_detector_header is not None: + return _core_load_detector_header(path, open_image_fn=fabio.open) + image = fabio.open(path) + try: + return dict(getattr(image, "header", None) or {}) + finally: + _close_detector_handle(image) + + +def _workbench_load_detector_pixels(path, *, dtype=np.float64): + return _workbench_load_detector_image(path, dtype=dtype).data + def _calibrated2d_package_paths(root_dir, sample_id): root = Path(root_dir) @@ -1468,7 +1516,7 @@ def _validate_existing_calibrated2d_package( ) try: - image_data = np.asarray(fabio.open(str(paths["image"])).data) + image_data = np.asarray(_workbench_load_detector_pixels(paths["image"], dtype=None)) except Exception as exc: raise ValueError(f"unreadable calibrated 2D EDF image: {paths['image']}: {exc}") from exc if image_data.ndim != 2 or image_data.size == 0 or not np.any(np.isfinite(image_data)): @@ -1479,7 +1527,7 @@ def _validate_existing_calibrated2d_package( except Exception as exc: raise ValueError(f"unreadable calibrated 2D NPY mask: {paths['mask_npy']}: {exc}") from exc try: - mask_edf = np.asarray(fabio.open(str(paths["mask_edf"])).data) + mask_edf = np.asarray(_workbench_load_detector_pixels(paths["mask_edf"], dtype=None)) except Exception as exc: raise ValueError(f"unreadable calibrated 2D EDF mask: {paths['mask_edf']}: {exc}") from exc if mask_npy.ndim != 2 or mask_edf.ndim != 2: @@ -2263,13 +2311,13 @@ def build_composite_bg_net( dark = np.asarray(d_dark, dtype=np.float64) for bg_path in bg_paths: - img = fabio.open(bg_path) - d_bg = np.asarray(img.data, dtype=np.float64) + loaded = _workbench_load_detector_image(bg_path) + d_bg = np.asarray(loaded.data, dtype=np.float64) self._assert_same_shape(d_bg, dark, "bg", "dark") if ref_shape is not None and tuple(d_bg.shape) != tuple(ref_shape): raise ValueError(f"BG 尺寸不匹配: {d_bg.shape} vs {ref_shape}") - exp, mon, trans = self.parse_header(bg_path, header_dict=getattr(img, "header", {})) + exp, mon, trans = self.parse_header(bg_path, header_dict=loaded.header) exp_use = exp if exp is not None else fallback_triplet[0] mon_use = mon if mon is not None else fallback_triplet[1] trans_use = trans if trans is not None else fallback_triplet[2] @@ -2998,8 +3046,8 @@ def has_keys(): need_text_fallback = not has_keys() else: try: - img = fabio.open(filepath) - for k, v in getattr(img, "header", {}).items(): + header = _workbench_load_detector_header(filepath) + for k, v in header.items(): add_meta(k, v) need_text_fallback = not has_keys() except Exception: @@ -3142,10 +3190,10 @@ def extract_instrument_signature(self, filepath, header_dict=None, shape=None): meta = self.normalize_header_dict(header_dict) if not meta: try: - img = fabio.open(filepath) - meta = self.normalize_header_dict(getattr(img, "header", {})) + loaded = _workbench_load_detector_image(filepath, dtype=None) + meta = self.normalize_header_dict(loaded.header) if shape is None: - shape = tuple(img.data.shape) + shape = tuple(loaded.data.shape) except Exception: pass @@ -3378,9 +3426,11 @@ def check_instrument_consistency(self, file_paths, poni_path=None, tol_pct=0.5): sigs = [] for fp in file_paths: try: - img = fabio.open(fp) - d = img.data - sig = self.extract_instrument_signature(fp, header_dict=getattr(img, "header", {}), shape=d.shape) + loaded = _workbench_load_detector_image(fp, dtype=None) + d = loaded.data + sig = self.extract_instrument_signature( + fp, header_dict=loaded.header, shape=d.shape + ) sigs.append(sig) except Exception as e: sigs.append({"path": str(fp), "shape": None, "error": str(e)}) @@ -3716,9 +3766,9 @@ def load_optional_array(self, path, name): return None p = Path(path) if p.suffix.lower() == ".npy": - arr = np.load(path) + arr = np.load(path, allow_pickle=False) else: - arr = fabio.open(path).data + arr = _workbench_load_detector_pixels(path, dtype=None) if arr is None: raise ValueError(f"{name} 文件无法读取: {path}") return np.asarray(arr) @@ -3753,9 +3803,9 @@ def build_reference_library(self, paths, *, return_rejections=False): rejected = [] for p in list(dict.fromkeys(paths or [])): try: - img = fabio.open(p) - data = np.asarray(img.data) - exp, mon, trans = self.parse_header(p, header_dict=getattr(img, "header", {})) + loaded = _workbench_load_detector_image(p, dtype=None) + data = np.asarray(loaded.data) + exp, mon, trans = self.parse_header(p, header_dict=loaded.header) refs.append({ "path": str(p), "shape": tuple(data.shape), @@ -7356,8 +7406,8 @@ def run_calibration(self): self.report(self.tr("rpt_solid_angle").format(state='ON' if apply_solid_angle else 'OFF')) ai = pyFAI.load(files["poni"]) - d_std = fabio.open(files["std"]).data.astype(np.float64) - d_dark = fabio.open(files["dark"]).data.astype(np.float64) + d_std = _workbench_load_detector_pixels(files["std"]) + d_dark = _workbench_load_detector_pixels(files["dark"]) self._assert_same_shape(d_std, d_dark, "std", "dark") dark_exposure_s = self.read_required_dark_exposure(files["dark"]) mask_path = self.global_vars["mask_path"].get().strip() @@ -7559,9 +7609,9 @@ def run_calibration(self): bg_transmissions = [] bg_exposures = [] for bg_path in bg_used_paths: - bg_image = fabio.open(bg_path) + bg_header = _workbench_load_detector_header(bg_path) bg_exp, bg_mon, bg_trans = self.parse_header( - bg_path, header_dict=getattr(bg_image, "header", {}) + bg_path, header_dict=bg_header ) bg_exp_use = bg_exp if bg_exp is not None else p["bg_exp"] bg_mon_use = bg_mon if bg_mon is not None else p["bg_i0"] @@ -8165,11 +8215,11 @@ def log_line(msg): def load_data(path): if context["parallel"]: - return fabio.open(path).data.astype(np.float64) + return _workbench_load_detector_pixels(path) with context["cache_lock"]: if path in context["image_cache"]: return context["image_cache"][path] - d = fabio.open(path).data.astype(np.float64) + d = _workbench_load_detector_pixels(path) with context["cache_lock"]: context["image_cache"][path] = d return d @@ -8270,9 +8320,9 @@ def load_data(path): return {"row": row, "logs": logs, "mode_stats": mode_stats} ai = context["ai_shared"] if not context["parallel"] else pyFAI.load(context["poni_path"]) - sample = fabio.open(fpath) - d_s = sample.data.astype(np.float64) - sample_header = getattr(sample, "header", {}) + loaded_sample = _workbench_load_detector_image(fpath) + d_s = np.asarray(loaded_sample.data, dtype=np.float64) + sample_header = loaded_sample.header exp, mon, trans = self.parse_header(fpath, header_dict=sample_header) monitor_mode = context["monitor_mode"] @@ -8893,7 +8943,7 @@ def prepare_batch_references(self, *, ref_mode, bg_path, dark_path, monitor_mode dark_text = str(dark_path or "").strip() if not bg_text or not dark_text: raise ValueError("固定参考模式缺少背景或暗场文件。") - fixed_dark_data = fabio.open(dark_text).data.astype(np.float64) + fixed_dark_data = _workbench_load_detector_pixels(dark_text) fixed_dark_exposure_s = self.read_required_dark_exposure(dark_text) bg_paths = self.split_path_list(bg_text) if not bg_paths: @@ -9949,13 +9999,13 @@ def dry_run(self): if self.t2_ref_mode.get() == "auto": try: - img = fabio.open(fp) + loaded = _workbench_load_detector_image(fp, dtype=None) smeta = { "exp": e if (e is not None and np.isfinite(e)) else None, "mon": m if m is not None else None, "trans": t if t is not None else None, "mtime": Path(fp).stat().st_mtime if Path(fp).exists() else None, - "shape": tuple(img.data.shape), + "shape": tuple(loaded.data.shape), } bg_ref, _, bg_rejected = self.select_best_reference( smeta, @@ -10114,7 +10164,7 @@ def _get_t2_preview_context(self): raise ValueError("请先在 Tab1/Tab2 设置 poni 文件。") ai = pyFAI.load(poni_path) - data = fabio.open(sample_path).data.astype(np.float64) + data = _workbench_load_detector_pixels(sample_path) if data.ndim != 2: raise ValueError(f"样品图像维度错误: {data.shape}") diff --git a/docs/architecture.md b/docs/architecture.md index f7321ab..d7813b5 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -113,10 +113,11 @@ ## Resource and publication boundaries -- FabIO handles are closed in the strict 1D readers and the strict 2D resume - verification reader. The shared reference loader, the strict 2D main image - loader, and multiple Workbench paths still require one common copy-and-close - loader. +- FabIO handles go through one common copy-and-close helper + (`saxsabs.io.detector_images`). The shared reference loader, the strict 2D + main and resume readers, the integrate1d mask/EDF loaders, and Workbench + detector-image paths copy pixels/header out and close the handle, including + after a read failure. Callers can inject `open_image_fn` at the I/O edge. - The reusable calibrated-2D package has transactional multi-file publication and signature-aware resume. Workbench existence-only resume is now disabled and hard-blocked, but no equivalent content-signature resume exists. The strict @@ -153,6 +154,6 @@ cancellable. Users should prefer headless workflows for unattended or large campaigns. `CAUTION` remains visible but is not separately persisted as an acknowledgement. -- **Input resources**: not every Workbench FabIO path uses one common - copy-and-close helper. The strict readers exercised by the headless workflows - close their handles, and reviewers should use the documented portable fixtures. +- **Input resources**: Workbench and headless detector readers share + `saxsabs.io.detector_images`. Reviewers should still use the documented + portable fixtures rather than private beamline files. diff --git a/examples/manual-verification.md b/examples/manual-verification.md index 3a186a5..46f863c 100644 --- a/examples/manual-verification.md +++ b/examples/manual-verification.md @@ -200,7 +200,9 @@ applicable: content-signature resume (existence-only resume must remain disabled); - Workbench preflight binds critical file content hashes and persists explicit CAUTION acceptance; -- all FabIO readers pass normal and exceptional close tests on Windows; +- all FabIO readers pass OS-level handle audits on every Windows workstation + (unit tests now cover the shared copy-and-close helper; a full desktop + handle audit remains a local check); - a cancellable background JobController keeps long jobs off the Tk UI thread. ## Notes diff --git a/src/saxsabs/core/reference_matching.py b/src/saxsabs/core/reference_matching.py index 2ca3f29..9cfad84 100644 --- a/src/saxsabs/core/reference_matching.py +++ b/src/saxsabs/core/reference_matching.py @@ -12,6 +12,8 @@ import numpy as np +from saxsabs.io.detector_images import load_detector_image + NO_USABLE_REFERENCE_SCORE = 1e9 DEFAULT_MAX_SCORE_THRESHOLD = 0.5 @@ -116,24 +118,13 @@ def build_reference_library( def parse_header_fn(p, header_dict=None): # type: ignore return None, None, None - if open_image_fn is None: - def _lazy_fabio_open(p): - import fabio - - return fabio.open(p) - - open_image_fn = _lazy_fabio_open - refs: list[dict[str, Any]] = [] rejected: list[dict[str, Any]] = [] for p in unique_paths: - img = None try: - img = open_image_fn(p) - raw_data = getattr(img, "data", None) - shape = tuple(np.asarray(raw_data).shape) if raw_data is not None else None - hdr = getattr(img, "header", {}) or {} - exp, mon, trans = parse_header_fn(p, header_dict=hdr) + loaded = load_detector_image(p, dtype=None, open_image_fn=open_image_fn) + shape = tuple(np.asarray(loaded.data).shape) + exp, mon, trans = parse_header_fn(p, header_dict=loaded.header) mtime = Path(p).stat().st_mtime if Path(p).exists() else None refs.append( { @@ -153,10 +144,6 @@ def _lazy_fabio_open(p): } ) continue - finally: - close = getattr(img, "close", None) if img is not None else None - if callable(close): - close() return (refs, rejected) if return_rejections else refs diff --git a/src/saxsabs/io/__init__.py b/src/saxsabs/io/__init__.py index 9d845e0..c1fe893 100644 --- a/src/saxsabs/io/__init__.py +++ b/src/saxsabs/io/__init__.py @@ -1,3 +1,10 @@ +from .detector_images import ( + DetectorImageLoad, + close_image_handle, + load_detector_header, + load_detector_image, + load_detector_pixels, +) from .parsers import ( extract_acquisition_timestamp, parse_header_values, @@ -18,6 +25,11 @@ ) __all__ = [ + "DetectorImageLoad", + "close_image_handle", + "load_detector_header", + "load_detector_image", + "load_detector_pixels", "parse_header_values", "parse_header_values_with_meta", "extract_acquisition_timestamp", diff --git a/src/saxsabs/io/detector_images.py b/src/saxsabs/io/detector_images.py new file mode 100644 index 0000000..b3f619c --- /dev/null +++ b/src/saxsabs/io/detector_images.py @@ -0,0 +1,119 @@ +"""Close-safe detector-image loading. + +FabIO image objects hold OS file handles. Every reader must copy pixels and +header out, then close the handle — including when the read later fails. +""" + +from __future__ import annotations + +from collections.abc import Callable, Mapping +from dataclasses import dataclass +from pathlib import Path +from typing import Any + +import numpy as np + +OpenImageFn = Callable[[str | Path], Any] + + +@dataclass(frozen=True) +class DetectorImageLoad: + """Owned detector pixels plus a copied header mapping.""" + + data: np.ndarray + header: dict[str, Any] + + +def close_image_handle(image: Any) -> None: + """Close a FabIO-like image if it exposes a callable ``close``.""" + + close = getattr(image, "close", None) + if callable(close): + close() + + +def copy_image_header(header: Any) -> dict[str, Any]: + """Return a plain dict copy of a FabIO-like header mapping.""" + + if header is None: + return {} + if isinstance(header, Mapping): + return dict(header) + try: + return dict(header) + except Exception: + return {} + + +def _default_open_image(path: str | Path) -> Any: + try: + import fabio + except ImportError as exc: + raise ImportError("fabio is required for detector-image reading") from exc + return fabio.open(str(path)) + + +def load_detector_image( + path: str | Path, + *, + dtype: np.dtype | type[np.generic] | None = np.float64, + open_image_fn: OpenImageFn | None = None, +) -> DetectorImageLoad: + """Open a detector image, copy pixels/header, and always close the handle. + + Parameters + ---------- + path + Image path passed to ``open_image_fn``. + dtype + Pixel dtype for the owned copy. ``None`` preserves the source dtype + (used for masks). A concrete dtype always copies into C-order. + open_image_fn + Injectable opener returning an object with ``.data``, optional + ``.header``, and optional ``.close``. Defaults to ``fabio.open``. + """ + + opener = open_image_fn if open_image_fn is not None else _default_open_image + image = opener(path) + try: + raw = getattr(image, "data", None) + if raw is None: + raise ValueError(f"detector image has no pixel data: {path}") + if dtype is None: + data = np.array(raw, copy=True) + else: + data = np.array(raw, dtype=dtype, copy=True, order="C") + header = copy_image_header(getattr(image, "header", None)) + return DetectorImageLoad(data=data, header=header) + finally: + close_image_handle(image) + + +def load_detector_pixels( + path: str | Path, + *, + dtype: np.dtype | type[np.generic] | None = np.float64, + open_image_fn: OpenImageFn | None = None, +) -> np.ndarray: + """Return owned detector pixels and close the image handle.""" + + return load_detector_image(path, dtype=dtype, open_image_fn=open_image_fn).data + + +def load_detector_header( + path: str | Path, + *, + open_image_fn: OpenImageFn | None = None, +) -> dict[str, Any]: + """Copy the image header and always close the handle. + + Header-only callers still go through the same close contract. Pixel data + is not required; a missing ``.data`` attribute is ignored. + """ + + opener = open_image_fn if open_image_fn is not None else _default_open_image + image = opener(path) + try: + return copy_image_header(getattr(image, "header", None)) + finally: + close_image_handle(image) diff --git a/src/saxsabs/workflows/bl19b2_abs2d.py b/src/saxsabs/workflows/bl19b2_abs2d.py index 5738215..72b1190 100644 --- a/src/saxsabs/workflows/bl19b2_abs2d.py +++ b/src/saxsabs/workflows/bl19b2_abs2d.py @@ -31,6 +31,7 @@ from saxsabs.core.normalization import compute_norm_factor as _core_compute_norm_factor from saxsabs.core.uncertainty import AbsoluteUncertaintyBudget, propagate_absolute_uncertainty from saxsabs.constants import get_reference_data +from saxsabs.io.detector_images import load_detector_image, load_detector_pixels SCHEMA_VERSION = "saxsabs.bl19b2_abs2d.v4" @@ -1744,18 +1745,11 @@ def find_reference_paths( def read_detector_image(path: str | Path) -> np.ndarray: - """Read a detector image using fabio and return float64 pixels.""" + """Read a detector image using the shared copy-and-close loader.""" try: - import fabio + return load_detector_pixels(path) except ImportError as exc: # pragma: no cover raise ImportError("fabio is required for BL19B2 detector image reading") from exc - image = fabio.open(str(path)) - try: - return np.array(image.data, dtype=np.float64, copy=True, order="C") - finally: - close = getattr(image, "close", None) - if callable(close): - close() def build_combined_mask( @@ -3893,63 +3887,60 @@ def _validate_existing_outputs(paths: OutputPaths, metadata: dict[str, Any]) -> _validate_resumed_array(h5_image, metadata=metadata, label="HDF5") try: - import fabio - - edf = fabio.open(str(paths.edf)) - try: - header = edf.header - if header.get("SAXSAbsSchema") != SCHEMA_VERSION: - raise ValueError("existing EDF internal schema mismatch") - if header.get("FormulaVersion") != FORMULA_VERSION: - raise ValueError("existing EDF formula version mismatch") - if header.get("ProcessingSignature") != metadata.get("processing_signature"): - raise ValueError("existing EDF processing signature mismatch") - if header.get("FrameSignature") != metadata.get("frame_signature"): - raise ValueError("existing EDF frame signature mismatch") - selection = metadata.get('sample_selection') or {} - derivation = metadata.get('thickness', {}).get('derivation') or {} - if header.get('IncludeManifestSHA256', '') != str(selection.get('sha256', '')): - raise ValueError('existing EDF include manifest checksum mismatch') - if header.get('ThicknessDerivationSHA256', '') != str( - derivation.get('sha256', '') - ): - raise ValueError('existing EDF thickness derivation checksum mismatch') - if header.get("IntensityUnit") != INTENSITY_UNIT: - raise ValueError("existing EDF intensity unit mismatch") - expected_k_headers = { - "KFactor": _edf_optional_float(external_k_contract["k_factor"]), - "KStd": _edf_optional_float(external_k_contract["k_std"]), - "KStdMeaning": str(external_k_contract["k_std_semantics"]).replace(";", ""), - "KStatStdU": _edf_optional_float( - external_k_contract["k_statistical_standard_uncertainty"] - ), - "KStandardU": _edf_optional_float( - external_k_contract["k_standard_uncertainty"] - ), - "KExpandedU": _edf_optional_float( - external_k_contract["k_expanded_uncertainty"] - ), - "KCoverage": _edf_optional_float(external_k_contract["coverage_factor"]), - } - for key, expected in expected_k_headers.items(): - if header.get(key) != expected: - raise ValueError(f"existing EDF K calibration uncertainty mismatch for {key}") - expected_uncertainty_status = str( - metadata.get("uncertainty", {}).get("status", "unknown") - ) - if header.get("UncertaintyStatus") != expected_uncertainty_status: - raise ValueError("existing EDF uncertainty status mismatch") - expected_expanded_status = str( - metadata.get("uncertainty", {}).get("expanded_status", "unavailable") - ) - if header.get("ExpandedUStatus") != expected_expanded_status: - raise ValueError("existing EDF expanded uncertainty status mismatch") - if Path(str(header.get("UncertaintyHDF5", ""))).resolve() != paths.h5.resolve(): - raise ValueError("existing EDF uncertainty HDF5 pointer mismatch") - edf_image = np.asarray(edf.data) - finally: - edf.close() - except (OSError, KeyError, TypeError) as exc: + loaded_edf = load_detector_image(paths.edf, dtype=None) + header = loaded_edf.header + edf_image = np.asarray(loaded_edf.data) + if header.get("SAXSAbsSchema") != SCHEMA_VERSION: + raise ValueError("existing EDF internal schema mismatch") + if header.get("FormulaVersion") != FORMULA_VERSION: + raise ValueError("existing EDF formula version mismatch") + if header.get("ProcessingSignature") != metadata.get("processing_signature"): + raise ValueError("existing EDF processing signature mismatch") + if header.get("FrameSignature") != metadata.get("frame_signature"): + raise ValueError("existing EDF frame signature mismatch") + selection = metadata.get('sample_selection') or {} + derivation = metadata.get('thickness', {}).get('derivation') or {} + if header.get('IncludeManifestSHA256', '') != str(selection.get('sha256', '')): + raise ValueError('existing EDF include manifest checksum mismatch') + if header.get('ThicknessDerivationSHA256', '') != str( + derivation.get('sha256', '') + ): + raise ValueError('existing EDF thickness derivation checksum mismatch') + if header.get("IntensityUnit") != INTENSITY_UNIT: + raise ValueError("existing EDF intensity unit mismatch") + expected_k_headers = { + "KFactor": _edf_optional_float(external_k_contract["k_factor"]), + "KStd": _edf_optional_float(external_k_contract["k_std"]), + "KStdMeaning": str(external_k_contract["k_std_semantics"]).replace(";", ""), + "KStatStdU": _edf_optional_float( + external_k_contract["k_statistical_standard_uncertainty"] + ), + "KStandardU": _edf_optional_float( + external_k_contract["k_standard_uncertainty"] + ), + "KExpandedU": _edf_optional_float( + external_k_contract["k_expanded_uncertainty"] + ), + "KCoverage": _edf_optional_float(external_k_contract["coverage_factor"]), + } + for key, expected in expected_k_headers.items(): + if header.get(key) != expected: + raise ValueError(f"existing EDF K calibration uncertainty mismatch for {key}") + expected_uncertainty_status = str( + metadata.get("uncertainty", {}).get("status", "unknown") + ) + if header.get("UncertaintyStatus") != expected_uncertainty_status: + raise ValueError("existing EDF uncertainty status mismatch") + expected_expanded_status = str( + metadata.get("uncertainty", {}).get("expanded_status", "unavailable") + ) + if header.get("ExpandedUStatus") != expected_expanded_status: + raise ValueError("existing EDF expanded uncertainty status mismatch") + if Path(str(header.get("UncertaintyHDF5", ""))).resolve() != paths.h5.resolve(): + raise ValueError("existing EDF uncertainty HDF5 pointer mismatch") + except (OSError, KeyError, TypeError, ValueError) as exc: + if isinstance(exc, ValueError) and "existing EDF" in str(exc): + raise raise ValueError(f"existing EDF output is unreadable or incomplete: {paths.edf}") from exc _validate_resumed_array(edf_image, metadata=metadata, label="EDF") diff --git a/src/saxsabs/workflows/bl19b2_integrate1d.py b/src/saxsabs/workflows/bl19b2_integrate1d.py index 51d14dc..371b7bf 100644 --- a/src/saxsabs/workflows/bl19b2_integrate1d.py +++ b/src/saxsabs/workflows/bl19b2_integrate1d.py @@ -20,6 +20,8 @@ import numpy as np +from saxsabs.io.detector_images import load_detector_image, load_detector_pixels + SCHEMA_VERSION = "saxsabs.bl19b2_integrate1d.v1" SUCCESS_STATUSES = frozenset({"success", "processed", "skipped_existing", "success_existing"}) @@ -204,16 +206,9 @@ def _load_mask(path: Path) -> np.ndarray: value = np.load(path, allow_pickle=False) else: try: - import fabio + value = load_detector_pixels(path, dtype=None) except ImportError as exc: # pragma: no cover raise ImportError("fabio is required to read the BL19B2 EDF mask") from exc - image = fabio.open(str(path)) - try: - value = np.asarray(image.data) - finally: - close = getattr(image, "close", None) - if callable(close): - close() if value.ndim != 2 or not np.all(np.isfinite(value)): raise ValueError("integration mask must be a finite 2D array") return (np.asarray(value) != 0).astype(np.uint8) @@ -344,17 +339,11 @@ def _validate_metadata( def _load_validate_edf(item: _InputRow, metadata: dict[str, Any], mask: np.ndarray) -> np.ndarray: try: - import fabio + loaded = load_detector_image(item.edf, dtype=None) except ImportError as exc: # pragma: no cover raise ImportError("fabio is required for BL19B2 EDF integration") from exc - image_file = fabio.open(str(item.edf)) - try: - image = np.asarray(image_file.data) - header = image_file.header - finally: - close = getattr(image_file, "close", None) - if callable(close): - close() + image = np.asarray(loaded.data) + header = loaded.header if image.shape != mask.shape: raise ValueError(f"EDF/mask shape mismatch for {item.relative_path}: {image.shape} vs {mask.shape}") if header.get("ProcessingSignature") != item.processing_signature: diff --git a/tests/test_detector_images.py b/tests/test_detector_images.py new file mode 100644 index 0000000..e6cec5c --- /dev/null +++ b/tests/test_detector_images.py @@ -0,0 +1,144 @@ +"""Close-safe detector-image loader tests. + +These tests drive the shipped helper with injected fake handles. They do not +re-implement the close/copy contract and do not hard-code scientific values. +""" + +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import Mock + +import numpy as np +import pytest + +from saxsabs.io.detector_images import ( + close_image_handle, + load_detector_header, + load_detector_image, + load_detector_pixels, +) + + +def test_load_detector_image_closes_handle_and_returns_owned_array(): + source = np.array([[1, 2]], dtype=np.int16) + opened = SimpleNamespace(data=source, header={"ExposureTime": "1.0"}, close=Mock()) + + loaded = load_detector_image("frame.tif", open_image_fn=lambda path: opened) + source[0, 0] = 99 + opened.header["ExposureTime"] = "mutated" + + np.testing.assert_array_equal(loaded.data, np.array([[1.0, 2.0]])) + assert loaded.data.dtype == np.float64 + assert loaded.header == {"ExposureTime": "1.0"} + opened.close.assert_called_once_with() + + +def test_load_detector_image_closes_handle_after_missing_pixel_data(): + opened = SimpleNamespace(header={}, close=Mock()) + + with pytest.raises(ValueError, match="no pixel data"): + load_detector_image("empty.tif", open_image_fn=lambda path: opened) + + opened.close.assert_called_once_with() + + +def test_load_detector_image_closes_handle_after_data_access_failure(): + opened = SimpleNamespace(close=Mock()) + + class BrokenData: + @property + def data(self): + raise RuntimeError("broken detector data") + + header = {} + close = opened.close + + with pytest.raises(RuntimeError, match="broken detector data"): + load_detector_image("broken.tif", open_image_fn=lambda path: BrokenData()) + + opened.close.assert_called_once_with() + + +def test_load_detector_pixels_preserves_source_dtype_when_requested(): + source = np.array([[1, 0]], dtype=np.uint8) + opened = SimpleNamespace(data=source, header={}, close=Mock()) + + pixels = load_detector_pixels( + "mask.edf", + dtype=None, + open_image_fn=lambda path: opened, + ) + source[0, 0] = 7 + + np.testing.assert_array_equal(pixels, np.array([[1, 0]], dtype=np.uint8)) + assert pixels.dtype == np.uint8 + opened.close.assert_called_once_with() + + +def test_load_detector_header_closes_without_requiring_pixel_data(): + opened = SimpleNamespace(header={"Monitor": "100"}, close=Mock()) + + header = load_detector_header("header-only.tif", open_image_fn=lambda path: opened) + opened.header["Monitor"] = "mutated" + + assert header == {"Monitor": "100"} + opened.close.assert_called_once_with() + + +def test_load_detector_header_closes_after_header_copy_failure(): + opened = SimpleNamespace(close=Mock()) + + class BrokenHeader: + def items(self): + raise RuntimeError("broken header") + + def __iter__(self): + raise RuntimeError("broken header") + + broken = SimpleNamespace(header=BrokenHeader(), close=opened.close) + + header = load_detector_header("bad-header.tif", open_image_fn=lambda path: broken) + + assert header == {} + opened.close.assert_called_once_with() + + +def test_close_image_handle_ignores_objects_without_close(): + close_image_handle(SimpleNamespace(data=[[1]])) + + +def test_read_detector_image_delegates_to_shared_loader(monkeypatch): + from saxsabs.workflows import bl19b2_abs2d as bl19b2 + + opened = SimpleNamespace(data=np.array([[3, 4]], dtype=np.int16), close=Mock()) + fake_fabio = SimpleNamespace(open=lambda path: opened) + monkeypatch.setitem(__import__("sys").modules, "fabio", fake_fabio) + + image = bl19b2.read_detector_image("frame.tif") + opened.data[0, 0] = 99 + + np.testing.assert_array_equal(image, np.array([[3.0, 4.0]])) + assert image.dtype == np.float64 + opened.close.assert_called_once_with() + + +def test_default_opener_uses_injected_path(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + seen: list[str] = [] + + class FakeFabio: + @staticmethod + def open(path): + seen.append(str(path)) + return SimpleNamespace( + data=np.array([[8, 9]], dtype=np.float64), + header={"path": str(path)}, + close=Mock(), + ) + + monkeypatch.setitem(__import__("sys").modules, "fabio", FakeFabio) + image_path = tmp_path / "frame.tif" + loaded = load_detector_image(image_path) + + assert seen == [str(image_path)] + np.testing.assert_array_equal(loaded.data, [[8.0, 9.0]]) + assert loaded.header["path"] == str(image_path) diff --git a/tests/test_public_exports.py b/tests/test_public_exports.py index 01339b7..ce174b2 100644 --- a/tests/test_public_exports.py +++ b/tests/test_public_exports.py @@ -19,10 +19,15 @@ def test_core_reexports_recent_batch_helpers(): def test_io_reexports_header_meta_helpers(): import saxsabs.io as io + from saxsabs.io.detector_images import load_detector_image from saxsabs.io.parsers import extract_acquisition_timestamp, parse_header_values_with_meta assert io.parse_header_values_with_meta is parse_header_values_with_meta assert io.extract_acquisition_timestamp is extract_acquisition_timestamp + assert io.load_detector_image is load_detector_image + assert callable(io.load_detector_pixels) + assert callable(io.load_detector_header) + assert callable(io.close_image_handle) def test_top_level_reexports_scientific_safety_contracts(): diff --git a/tests/test_workbench_scientific.py b/tests/test_workbench_scientific.py index 6103ff8..6eeb4ca 100644 --- a/tests/test_workbench_scientific.py +++ b/tests/test_workbench_scientific.py @@ -3,6 +3,7 @@ from pathlib import Path import sys from types import SimpleNamespace +from unittest.mock import Mock import numpy as np import pytest @@ -2078,3 +2079,75 @@ def test_tab3_preflight_k_reader_handles_blank_default_without_crashing( else: assert value == pytest.approx(expected_value) assert message == (app.tr(message_key) if message_key is not None else None) + + +def test_workbench_parse_header_closes_fabio_handle_and_reads_values(tmp_path, monkeypatch): + module = _load_workbench_module() + app = module.SAXSAbsWorkbenchApp.__new__(module.SAXSAbsWorkbenchApp) + path = tmp_path / "frame.tif" + path.write_bytes(b"placeholder") + opened = SimpleNamespace( + header={"ExposureTime": "1.0", "Monitor": "100", "Transmission": "0.8"}, + data=np.ones((2, 2)), + close=Mock(), + ) + monkeypatch.setattr(module.fabio, "open", lambda _path: opened) + + exp, mon, trans = app.parse_header(str(path)) + + assert exp == pytest.approx(1.0) + assert mon == pytest.approx(100.0) + assert trans == pytest.approx(0.8) + opened.close.assert_called_once_with() + + +def test_workbench_load_optional_array_closes_handle_and_returns_owned_copy( + tmp_path, monkeypatch +): + module = _load_workbench_module() + app = module.SAXSAbsWorkbenchApp.__new__(module.SAXSAbsWorkbenchApp) + path = tmp_path / "mask.edf" + path.write_bytes(b"placeholder") + original = np.array([[1, 0]], dtype=np.uint8) + opened = SimpleNamespace(data=original, header={}, close=Mock()) + monkeypatch.setattr(module.fabio, "open", lambda _path: opened) + + arr = app.load_optional_array(str(path), "Mask") + original[0, 0] = 99 + + np.testing.assert_array_equal(arr, [[1, 0]]) + opened.close.assert_called_once_with() + + +def test_workbench_load_optional_array_closes_handle_after_read_failure( + tmp_path, monkeypatch +): + module = _load_workbench_module() + app = module.SAXSAbsWorkbenchApp.__new__(module.SAXSAbsWorkbenchApp) + path = tmp_path / "mask.edf" + path.write_bytes(b"placeholder") + opened = SimpleNamespace(close=Mock()) + + class BrokenData: + @property + def data(self): + raise RuntimeError("broken mask data") + + header = {} + close = opened.close + + monkeypatch.setattr(module.fabio, "open", lambda _path: BrokenData()) + + with pytest.raises(RuntimeError, match="broken mask data"): + app.load_optional_array(str(path), "Mask") + opened.close.assert_called_once_with() + + +def test_workbench_load_optional_array_refuses_pickled_npy(tmp_path): + module = _load_workbench_module() + app = module.SAXSAbsWorkbenchApp.__new__(module.SAXSAbsWorkbenchApp) + path = tmp_path / "mask.npy" + np.save(path, np.array([{"unsafe": True}], dtype=object), allow_pickle=True) + + with pytest.raises(ValueError): + app.load_optional_array(str(path), "Mask")