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()