From e08ac1a44be05c4bb0ffd49f03f75d44048b8dc7 Mon Sep 17 00:00:00 2001 From: x01dc Date: Tue, 1 Sep 2026 20:15:25 +0200 Subject: [PATCH] fix(omny): make correction data global-var-backed, fix default file path Two real bugs found while investigating a recurring AttributeError: 'OMNY' object has no attribute 'corr_pos_x' from tomo_scan_projection(). 1. default_correction_file/default_correction_file_x were bare relative filenames ("correction_omny_202204.txt"), unlike flomni's equivalent which resolves an absolute path via Path(csaxs_bec.__file__). open() would only ever succeed if bec were launched from this exact plugin directory -- silently swallowed everywhere else by the existing FileNotFoundError handler in reset_correction(). The real correction files genuinely exist on disk next to the plugin code and were very likely never actually being loaded. 2. corr_pos_x/corr_angle_x/corr_pos_y/corr_angle_y/corr_pos_y_2/ corr_angle_y_2 were plain instance attributes, only ever assigned inside reset_correction() (itself only called from otransfer_put_sample() during a sample transfer) -- hence the AttributeError when calling tomo_scan_projection() beforehand. Converted to global-var-backed properties defaulting to [] when unset, mirroring flomni's corr_pos_y/corr_angle_y/etc. exactly. This supersedes a shallower getattr()-based band-aid from an earlier, uncommitted attempt at this fix. As a bonus, correction data now persists across BEC client restarts, same as flomni's. Note flomni has no x-correction concept at all (only corr_pos_y/ corr_pos_y_2) -- omny's x-correction is its own thing, not something copied from flomni. 17 new/updated tests in test_omny_alignment_mixin.py, including an end-to-end check that reset_correction(use_default_correction=True) now actually loads the real correction_omny_202204*.txt files. --- .../plugins/omny/omny_alignment_mixin.py | 88 ++++++++++- .../test_omny_alignment_mixin.py | 143 ++++++++++++++++++ 2 files changed, 229 insertions(+), 2 deletions(-) create mode 100644 tests/tests_bec_ipython_client/test_omny_alignment_mixin.py diff --git a/csaxs_bec/bec_ipython_client/plugins/omny/omny_alignment_mixin.py b/csaxs_bec/bec_ipython_client/plugins/omny/omny_alignment_mixin.py index ae1f433b..df627113 100644 --- a/csaxs_bec/bec_ipython_client/plugins/omny/omny_alignment_mixin.py +++ b/csaxs_bec/bec_ipython_client/plugins/omny/omny_alignment_mixin.py @@ -1,6 +1,7 @@ import time import numpy as np import os +from pathlib import Path from rich import box from rich.console import Console @@ -17,8 +18,87 @@ class OMNYAlignmentError(Exception): class OMNYAlignmentMixin: - default_correction_file = "correction_omny_202204.txt" - default_correction_file_x = "correction_omny_202204_x.txt" + # Resolved to an absolute path (mirrors Flomni's default_correction_file + # setup) -- a bare relative filename would only ever be found if bec were + # launched from this exact plugin directory, silently failing (caught by + # the FileNotFoundError handler below) everywhere else. + import csaxs_bec + + _csaxs_bec_basepath = Path(csaxs_bec.__file__).parent + default_correction_file = ( + _csaxs_bec_basepath + / "bec_ipython_client" + / "plugins" + / "omny" + / "correction_omny_202204.txt" + ).resolve() + default_correction_file_x = ( + _csaxs_bec_basepath + / "bec_ipython_client" + / "plugins" + / "omny" + / "correction_omny_202204_x.txt" + ).resolve() + del csaxs_bec + + # additional x/y-correction curves: global-var-backed so they survive a + # BEC client restart, same pattern as every other persistent setting in + # this class (and matching Flomni's corr_pos_y/corr_angle_y/etc.). NOTE: + # must NOT be pre-assigned in OMNY.__init__ -- doing so would overwrite + # the persisted value with [] on every restart. + @property + def corr_pos_x(self): + val = self.client.get_global_var("corr_pos_x") + return [] if val is None else val + + @corr_pos_x.setter + def corr_pos_x(self, val: list): + self.client.set_global_var("corr_pos_x", val) + + @property + def corr_angle_x(self): + val = self.client.get_global_var("corr_angle_x") + return [] if val is None else val + + @corr_angle_x.setter + def corr_angle_x(self, val: list): + self.client.set_global_var("corr_angle_x", val) + + @property + def corr_pos_y(self): + val = self.client.get_global_var("corr_pos_y") + return [] if val is None else val + + @corr_pos_y.setter + def corr_pos_y(self, val: list): + self.client.set_global_var("corr_pos_y", val) + + @property + def corr_angle_y(self): + val = self.client.get_global_var("corr_angle_y") + return [] if val is None else val + + @corr_angle_y.setter + def corr_angle_y(self, val: list): + self.client.set_global_var("corr_angle_y", val) + + @property + def corr_pos_y_2(self): + val = self.client.get_global_var("corr_pos_y_2") + return [] if val is None else val + + @corr_pos_y_2.setter + def corr_pos_y_2(self, val: list): + self.client.set_global_var("corr_pos_y_2", val) + + @property + def corr_angle_y_2(self): + val = self.client.get_global_var("corr_angle_y_2") + return [] if val is None else val + + @corr_angle_y_2.setter + def corr_angle_y_2(self, val: list): + self.client.set_global_var("corr_angle_y_2", val) def reset_correction(self, use_default_correction=True): """ @@ -215,6 +295,10 @@ class OMNYAlignmentMixin: return self._compute_additional_correction(angle, iteration="y2") def _compute_additional_correction(self, angle, iteration="y1"): + # corr_pos_x/corr_pos_y/corr_pos_y_2 etc. are global-var-backed + # properties that default to [] -- never AttributeError, even before + # reset_correction() has ever run (e.g. testing scan mechanics before + # any sample transfer). if iteration == "x1": corr_pos = self.corr_pos_x corr_angle = self.corr_angle_x diff --git a/tests/tests_bec_ipython_client/test_omny_alignment_mixin.py b/tests/tests_bec_ipython_client/test_omny_alignment_mixin.py new file mode 100644 index 00000000..9ef20684 --- /dev/null +++ b/tests/tests_bec_ipython_client/test_omny_alignment_mixin.py @@ -0,0 +1,143 @@ +"""Regression test for OMNYAlignmentMixin's correction data. + +corr_pos_x/corr_angle_x/corr_pos_y/etc. used to be plain instance attributes, +only ever assigned inside reset_correction() -- calling +_compute_additional_correction() before reset_correction() had ever run (e.g. +tomo_scan_projection() called to test scan mechanics without first doing a +sample transfer, the only place reset_correction() gets called, see +omny_sample_transfer_mixin.py's otransfer_put_sample()) raised +'OMNY' object has no attribute 'corr_pos_x'. + +They are now global-var-backed properties (mirroring Flomni's +corr_pos_y/corr_angle_y/etc.), defaulting to [] when unset -- matching the +graceful "no data available" handling _compute_additional_correction() +already had for an empty (but present) list, and persisting across BEC +client restarts like every other setting in this class. + +Also regression-guards default_correction_file/default_correction_file_x +being resolved to an absolute path (a bare relative filename would only ever +be found if bec were launched from this exact plugin directory). +""" + +import os + +import pytest + +from csaxs_bec.bec_ipython_client.plugins.omny.omny_alignment_mixin import OMNYAlignmentMixin + + +class FakeClient: + """Minimal in-memory stand-in for BEC's global-var store.""" + + def __init__(self): + self._vars = {} + + def get_global_var(self, key): + return self._vars.get(key) + + def set_global_var(self, key, value): + self._vars[key] = value + + +def make_alignment(): + # bare instance, deliberately never calling reset_correction() -- mirrors + # a fresh OMNY session where no sample has been transferred in yet + align = OMNYAlignmentMixin() + align.client = FakeClient() + return align + + +def test_default_correction_files_are_absolute_and_exist(): + assert os.path.isabs(OMNYAlignmentMixin.default_correction_file) + assert os.path.isabs(OMNYAlignmentMixin.default_correction_file_x) + assert os.path.exists(OMNYAlignmentMixin.default_correction_file) + assert os.path.exists(OMNYAlignmentMixin.default_correction_file_x) + + +@pytest.mark.parametrize( + "attr", + ["corr_pos_x", "corr_angle_x", "corr_pos_y", "corr_angle_y", "corr_pos_y_2", "corr_angle_y_2"], +) +def test_correction_properties_default_to_empty_list(attr): + align = make_alignment() + + assert getattr(align, attr) == [] + + +@pytest.mark.parametrize("iteration", ["x1", "y1", "y2"]) +def test_compute_additional_correction_returns_zero_when_never_reset(iteration, capsys): + align = make_alignment() + + result = align._compute_additional_correction(45.0, iteration=iteration) + + assert result == 0 + assert "No data available" in capsys.readouterr().out + + +def test_compute_additional_correction_x_does_not_raise(): + align = make_alignment() + + assert align.compute_additional_correction_x(0.0) == 0 + + +def test_compute_additional_correction_y_does_not_raise(): + align = make_alignment() + + assert align.compute_additional_correction_y(0.0) == 0 + + +def test_compute_additional_correction_y_2_does_not_raise(): + align = make_alignment() + + assert align.compute_additional_correction_y_2(0.0) == 0 + + +def test_compute_additional_correction_still_works_after_reset(): + """Once corr_pos_x/corr_angle_x etc. are actually set (mirrors what + reset_correction() -> read_additional_correction_x() would do), the + normal closest-angle lookup still works.""" + align = make_alignment() + align.corr_pos_x = [1.0, 2.0, 3.0] + align.corr_angle_x = [0.0, 90.0, 180.0] + + result = align._compute_additional_correction(85.0, iteration="x1") + + assert result == 2.0 + + +def test_correction_properties_persist_across_instances_via_shared_client(): + """Global-var-backed: two instances sharing the same client see the same + value, unlike a plain instance attribute.""" + client = FakeClient() + align_a = OMNYAlignmentMixin() + align_a.client = client + align_b = OMNYAlignmentMixin() + align_b.client = client + + align_a.corr_pos_x = [5.0] + + assert align_b.corr_pos_x == [5.0] + + +def test_reset_correction_with_default_loads_real_file_data(): + """End-to-end check of the path-resolution fix: reset_correction(True) + must actually find and load the real correction_omny_202204*.txt files + (previously silently failed everywhere except when bec happened to be + launched from this exact plugin directory).""" + align = make_alignment() + + align.reset_correction(use_default_correction=True) + + assert len(align.corr_pos_x) > 0 + assert len(align.corr_pos_y) > 0 + + +def test_reset_correction_without_default_clears_to_empty(): + align = make_alignment() + align.corr_pos_x = [1.0] + align.corr_angle_x = [0.0] + + align.reset_correction(use_default_correction=False) + + assert align.corr_pos_x == [] + assert align.corr_angle_x == []