fix(omny): make correction data global-var-backed, fix default file path
CI for csaxs_bec / test (push) Successful in 2m33s
CI for csaxs_bec / test (push) Successful in 2m33s
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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 == []
|
||||
Reference in New Issue
Block a user