From 6543d247d97c5201c88f574cc15e12e64bc4262e Mon Sep 17 00:00:00 2001 From: Kristian Date: Tue, 27 Oct 2026 01:26:04 +0100 Subject: [PATCH] report the reference images of the num_ref_images window that hold no results The reference images named by ref_img_indices were checked for intensity values, the first num_ref_images ones were not. An image that could not be read keeps its ID in the image infos but produces no intensities, so a skipped image inside the reference window dropped out of the average unnoticed. With num_ref_images = 1 and no results for image 1 the reference intensities came back empty and the analysis normalised by nothing. Both paths now request an explicit list of image IDs and hand it to _check_requested_ref_imgs_available, which keeps the behaviour the named reference images already had: a partially missing window warns and is averaged over the remaining images, an empty one raises. The raised message names num_ref_images and ref_img_indices as the settings to change. The reference window is not slid to the next available images on purpose. The window has to precede the experiment, so moving it would pull an image that may already show smoke into the reference. Co-Authored-By: Claude Opus 5 --- ledsa/analysis/ExtinctionCoefficients.py | 23 ++++++-- .../07_test_corrupted_images.robot | 16 +++++- .../AcceptanceTests/LedsaATestLibrary.py | 35 ++++++++++++ .../UnitTests/test_reference_intensities.py | 55 +++++++++++++++++-- 4 files changed, 118 insertions(+), 11 deletions(-) diff --git a/ledsa/analysis/ExtinctionCoefficients.py b/ledsa/analysis/ExtinctionCoefficients.py index 8adb411..e663e8b 100644 --- a/ledsa/analysis/ExtinctionCoefficients.py +++ b/ledsa/analysis/ExtinctionCoefficients.py @@ -1,6 +1,7 @@ import warnings from abc import ABC, abstractmethod from multiprocessing import Pool +from typing import List import numpy as np import pandas as pd @@ -195,12 +196,17 @@ def calc_and_set_ref_intensities(self) -> None: """ Calculate and set the reference intensities for all LEDs based on the reference images. + :raises ValueError: if none of the reference images holds LED data. """ if self.ref_img_indices is not None: + requested_img_ids = list(self.ref_img_indices) ref_img_data = self.calculated_img_data.query(f'img_id == {self.ref_img_indices}') - self._check_requested_ref_imgs_available(ref_img_data) else: + # The reference window is the first num_ref_imgs images, whether or not they were analysed, + # so a skipped image among them is missing from the average and has to be reported + requested_img_ids = list(range(1, self.num_ref_imgs + 1)) ref_img_data = self.calculated_img_data.query(f'img_id <= {self.num_ref_imgs}') + self._check_requested_ref_imgs_available(ref_img_data, requested_img_ids) print( f"Images with indices {ref_img_data.index.get_level_values('img_id').unique().values} were used for calculating reference intensities.") @@ -208,27 +214,32 @@ def calc_and_set_ref_intensities(self) -> None: check_intensity_normalization(ref_img_data, ref_intensities, self.reference_property) self.ref_intensities = ref_intensities.to_numpy() - def _check_requested_ref_imgs_available(self, ref_img_data: pd.DataFrame) -> None: + def _check_requested_ref_imgs_available(self, ref_img_data: pd.DataFrame, + requested_img_ids: List[int]) -> None: """ - Report the reference images of :attr:`ref_img_indices` that hold no LED data. + Report the requested reference images that hold no LED data. Images skipped during the intensity extraction keep their ID in the image infos but produce no intensities, so a reference image ID naming one of them would silently - drop out of the average instead of being noticed. + drop out of the average instead of being noticed. This holds for the images named by + :attr:`ref_img_indices` and for the first :attr:`num_ref_imgs` images alike. :param ref_img_data: Intensity data of the requested reference images. :type ref_img_data: pd.DataFrame + :param requested_img_ids: IDs of the reference images the configuration asks for. + :type requested_img_ids: List[int] :raises ValueError: if none of the requested reference images holds LED data. """ available = set(ref_img_data.index.get_level_values('img_id').unique()) - missing = [img_id for img_id in self.ref_img_indices if img_id not in available] + missing = [img_id for img_id in requested_img_ids if img_id not in available] if not missing: return msg = (f'The reference images {missing} of LED array {self.experiment.led_array} in ' f'{self.experiment.path} hold no intensity values. They were either skipped during ' f'the extraction of the LED intensities or they lie outside the analysed images.') if not available: - raise ValueError(f'{msg} No reference intensities can be calculated.') + raise ValueError(f'{msg} No reference intensities can be calculated. Set num_ref_images or ' + f'ref_img_indices in config_analysis.ini to images that hold results.') warnings.warn(f'{msg} The reference intensities are averaged over the remaining images ' f'{sorted(available)}.', category=UserWarning, stacklevel=2) diff --git a/ledsa/tests/AcceptanceTests/07_test_corrupted_images.robot b/ledsa/tests/AcceptanceTests/07_test_corrupted_images.robot index cb33ec7..61c7f4f 100644 --- a/ledsa/tests/AcceptanceTests/07_test_corrupted_images.robot +++ b/ledsa/tests/AcceptanceTests/07_test_corrupted_images.robot @@ -49,10 +49,24 @@ Analysis Reads The Column Names From The First Existing Result File Corrupted Images Report Should List test_img_1.jpg extraction of the LED intensities # image 1 is the default reference image and has no results here, so image 2 provides the reference Create And Fill Config Analysis linear num_ref_images=2 - Execute Ledsa --analysis + ${stderr} = Execute Ledsa Expecting Warning --analysis + # the reference window of two images still names image 1, whose absence is reported + Should Contain ${stderr} reference images [1] Analysis Should Exclude Image test_img_1.jpg Extco Time Axis Should Be Accepted +Analysis Without Any Reference Image Results Stops With An Error + [Documentation] Image 1 is the only reference image of the default configuration. If it could not + ... be read, no reference intensities can be calculated, which has to be reported + ... instead of leaving the reference empty and normalising by nothing. + Corrupt Image test_img_1.jpg truncate + Execute Ledsa -s3_fast + Corrupted Images Report Should List test_img_1.jpg extraction of the LED intensities + Create And Fill Config Analysis linear + ${stderr} = Execute Ledsa Expecting Failure --analysis + Should Contain ${stderr} hold no intensity values + Should Contain ${stderr} num_ref_images or ref_img_indices + *** Keywords *** Restore Test Images Restore Corrupted Images diff --git a/ledsa/tests/AcceptanceTests/LedsaATestLibrary.py b/ledsa/tests/AcceptanceTests/LedsaATestLibrary.py index bc14501..fcb6247 100644 --- a/ledsa/tests/AcceptanceTests/LedsaATestLibrary.py +++ b/ledsa/tests/AcceptanceTests/LedsaATestLibrary.py @@ -454,6 +454,29 @@ def execute_ledsa(self, arg, inp=None): out = wait_for_process_to_finish(p, inp) return out + @keyword + def execute_ledsa_expecting_failure(self, arg): + """Run ledsa and return its stderr, for a run that is supposed to stop with an error. + + execute_ledsa hands its output to check_error_msg, which aborts the whole suite as soon as + anything reaches stderr, so a test of a reported error cannot go through it. + """ + returncode, stderr = run_ledsa_capturing_stderr(arg) + if returncode == 0: + raise AssertionError(f'ledsa {arg} succeeded, but it was expected to stop with an error.') + return stderr + + @keyword + def execute_ledsa_expecting_warning(self, arg): + """Run ledsa to completion and return its stderr, for a run that is supposed to warn. + + A warning reaches stderr as well, which execute_ledsa would treat as a failed suite. + """ + returncode, stderr = run_ledsa_capturing_stderr(arg) + if returncode != 0: + raise AssertionError(f'ledsa {arg} stopped with an error:\n{stderr}') + return stderr + @keyword def create_cc_matrix_file(self): file = open("mean_all_cc_matrix_integral.csv", "w", encoding="utf-8") @@ -739,6 +762,18 @@ def _read_extco_with_warnings(): return [str(entry.message) for entry in caught if entry.category is UserWarning] +def run_ledsa_capturing_stderr(arg): + """Run ledsa and return its exit code and stderr, without the stderr check of execute_ledsa. + + :param arg: Command line argument ledsa is called with. + :return: Exit code and decoded stderr of the run. + :rtype: Tuple[int, str] + """ + p = Popen(['python', '-m', 'ledsa', arg], stdin=PIPE, stdout=PIPE, stderr=PIPE) + _, stderr = p.communicate() + return p.returncode, stderr.decode('utf-8') + + def wait_for_process_to_finish(p, inp=None): out = p.communicate(inp) check_error_msg(out) diff --git a/ledsa/tests/UnitTests/test_reference_intensities.py b/ledsa/tests/UnitTests/test_reference_intensities.py index 592c6c0..bc9ccb9 100644 --- a/ledsa/tests/UnitTests/test_reference_intensities.py +++ b/ledsa/tests/UnitTests/test_reference_intensities.py @@ -1,4 +1,4 @@ -"""Unit tests for the reference intensities computed from explicitly named reference images.""" +"""Unit tests for the reference intensities, both from explicitly named images and from the first ones.""" from pathlib import Path import numpy as np @@ -23,15 +23,18 @@ class ExperimentStub: path = Path('.') -def make_solver(available_img_ids, ref_img_indices, intensity_of_img=lambda img_id: 100.0): +def make_solver(available_img_ids, ref_img_indices=None, num_ref_imgs=1, + intensity_of_img=lambda img_id: 100.0): """Create a solver whose image data holds the given images only. :param available_img_ids: IDs of the images that hold intensity values. - :param ref_img_indices: Reference image IDs requested by the configuration. + :param ref_img_indices: Reference image IDs requested by the configuration. None falls back to + the first *num_ref_imgs* images. + :param num_ref_imgs: Size of the reference window used when no indices are requested. :param intensity_of_img: Intensity of every LED of an image. """ solver = ExtinctionCoefficientsStub(ExperimentStub(), reference_property=REFERENCE_PROPERTY, - num_ref_imgs=1, ref_img_indices=ref_img_indices) + num_ref_imgs=num_ref_imgs, ref_img_indices=ref_img_indices) index = pd.MultiIndex.from_product([available_img_ids, LED_IDS], names=['img_id', 'led_id']) solver.calculated_img_data = pd.DataFrame( {REFERENCE_PROPERTY: [intensity_of_img(img_id) for img_id, _ in index]}, index=index) @@ -65,3 +68,47 @@ def test_reference_images_without_any_data_raise(): with pytest.raises(ValueError, match=r'\[4, 5\]'): solver.calc_and_set_ref_intensities() + + +class TestReferenceWindowOfTheFirstImages: + """The reference window of num_ref_imgs is checked like explicitly named reference images.""" + + def test_complete_window_is_averaged_without_a_warning(self, recwarn): + solver = make_solver([1, 2, 3], num_ref_imgs=2, + intensity_of_img=lambda img_id: 100.0 + 2.0 * img_id) + + solver.calc_and_set_ref_intensities() + + assert list(solver.ref_intensities) == [103.0] * len(LED_IDS) + assert [str(warning.message) for warning in recwarn] == [] + + def test_unreadable_first_image_raises_instead_of_leaving_the_reference_empty(self): + # The reported case: image 1 is the only reference image and could not be read, which used to + # leave the reference intensities empty without a word + solver = make_solver([2, 3, 4], num_ref_imgs=1) + + with pytest.raises(ValueError, match=r'\[1\]'): + solver.calc_and_set_ref_intensities() + + def test_unreadable_first_image_of_a_larger_window_warns(self): + solver = make_solver([2, 3], num_ref_imgs=3) + + with pytest.warns(UserWarning, match=r'\[1\]'): + solver.calc_and_set_ref_intensities() + + # The remaining images of the window still give reference intensities + assert list(solver.ref_intensities) == [100.0] * len(LED_IDS) + + def test_window_reaching_past_the_analysed_images_warns(self): + solver = make_solver([1, 2], num_ref_imgs=4) + + with pytest.warns(UserWarning, match=r'\[3, 4\]'): + solver.calc_and_set_ref_intensities() + + assert list(solver.ref_intensities) == [100.0] * len(LED_IDS) + + def test_raised_message_names_the_settings_to_change(self): + solver = make_solver([2, 3], num_ref_imgs=1) + + with pytest.raises(ValueError, match='num_ref_images or ref_img_indices'): + solver.calc_and_set_ref_intensities()