From 48adba8ff600d391924883192cd364fd90097d92 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Tue, 30 Jun 2026 16:02:46 +0100 Subject: [PATCH 01/10] Replace '_is_manual_autotem' with '_get_project_name' --- src/murfey/client/contexts/fib.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/murfey/client/contexts/fib.py b/src/murfey/client/contexts/fib.py index fa8ae9fb4..222c9d027 100644 --- a/src/murfey/client/contexts/fib.py +++ b/src/murfey/client/contexts/fib.py @@ -182,18 +182,19 @@ def _parse_boolean(text: str): ) -def _is_manual_autotem(file_path: Path): +def _get_project_name(file_path: Path): """ - Checks if the file being analysed belongs to a manual AutoTEM project. - Manual AutoTEM projects are stored in project folders that start with 'AutoTEM_'. + Get the project name from the file path. This is used in manual AutoTEM + workflows to identify the folder containing the images and site metadata + to register. """ try: autotem_idx = file_path.parts.index("autotem") project_dir = file_path.parents[-(autotem_idx + 2)] - return project_dir.stem.startswith("AutoTEM_") + return project_dir.stem except Exception: logger.error( - f"Error checking if file {file_path} belongs to a manual AutoTEM project:", + f"Error extracting project name from file path {file_path}:", exc_info=True, ) return None @@ -263,12 +264,16 @@ def post_transfer( # AutoTEM # ----------------------------------------------------------------------------- if self._acquisition_software == "autotem": - # Apply different logic depending of whether it's auto/manual AutoTEM - if (is_manual_autotem := _is_manual_autotem(transferred_file)) is None: + # Extract project name from file path + project_name = _get_project_name(transferred_file) + + # Handle file based on extracted project name + if project_name is None: # Skip processing file if the check fails return None - elif is_manual_autotem: - # Logic for handling manual AutotTEM will evenutlaly go here + # Manual AutoTEM folders start with + elif project_name.startswith("AutoTEM_"): + # Logic for handling manual AutoTEM will eventually go here return None else: # Extract metadata from fully automated AutoTEM projects From 84aa42779bc7bb1499b85d16653e8e805a114be9 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Wed, 1 Jul 2026 10:52:31 +0100 Subject: [PATCH 02/10] Modified the FIBContext logic so that file-based processing is only triggered for AutoTEM project folders containing relevant files --- src/murfey/client/contexts/fib.py | 62 ++++++++++++++++++++++++------- 1 file changed, 49 insertions(+), 13 deletions(-) diff --git a/src/murfey/client/contexts/fib.py b/src/murfey/client/contexts/fib.py index 222c9d027..ace16e9f6 100644 --- a/src/murfey/client/contexts/fib.py +++ b/src/murfey/client/contexts/fib.py @@ -246,6 +246,8 @@ def __init__( super().__init__("FIBContext", acquisition_software, token) self._basepath = basepath self._machine_config = machine_config + self._project_data: dict[str, Path] = {} + self._target_projects: list[str] = [] self._site_info: dict[int, LamellaSiteInfo] = {} self._drift_correction_images: dict[int, FIBImage] = {} @@ -264,24 +266,58 @@ def post_transfer( # AutoTEM # ----------------------------------------------------------------------------- if self._acquisition_software == "autotem": - # Extract project name from file path + # Extract current project name from file path project_name = _get_project_name(transferred_file) - - # Handle file based on extracted project name if project_name is None: - # Skip processing file if the check fails - return None - # Manual AutoTEM folders start with - elif project_name.startswith("AutoTEM_"): - # Logic for handling manual AutoTEM will eventually go here + # Early exit if the check fails return None - else: - # Extract metadata from fully automated AutoTEM projects - if transferred_file.name == "ProjectData.dat": - logger.info(f"Found metadata file {transferred_file} for parsing") + + # Store incoming ProjectData.dat files in memory + if ( + transferred_file.name == "ProjectData.dat" + and self._project_data.get(project_name) is None + ): + self._project_data[project_name] = transferred_file + + # Identify if the current file's project is to be registered + if project_name not in self._target_projects: + if not any( + pattern in str(transferred_file) + for pattern in ( + "/DCImages/", + "/LamellaEvaluationImages/", + "/Sites/Lamella", + ) + ): + return None + self._target_projects.append(project_name) + logger.info( + f"AutoTEM project {project_name!r} identified for registration" + ) + + # Analyse file and trigger processing if it passes the check + if project_name in self._target_projects: + # Extract metadata from valid AutoTEM projects + if transferred_file.name == "ProjectData.dat" or ( + self._project_data.get(project_name) and not self._site_info + ): + # Extract directly from incoming file + if transferred_file.name == "ProjectData.dat": + logger.info( + f"Found metadata file {transferred_file} for parsing" + ) + all_site_info_new = self._parse_autotem_metadata( + transferred_file + ) + # Extract for the first time from stored file path + else: + project_data = self._project_data[project_name] + logger.info( + f"Performing initial metadata extraction from {project_data}" + ) + all_site_info_new = self._parse_autotem_metadata(project_data) # Parse the metadata file - all_site_info_new = self._parse_autotem_metadata(transferred_file) for site_num, site_info_new in all_site_info_new.items(): # Post the data to the backend if it's been changed if ( From b5d6e7e0ff43b72a38c1aed8b4b19c0d8477f3db Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Wed, 1 Jul 2026 10:54:34 +0100 Subject: [PATCH 03/10] Updated FIBContext tests to check new logic --- tests/client/contexts/test_fib.py | 46 +++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/tests/client/contexts/test_fib.py b/tests/client/contexts/test_fib.py index c9fc019c1..ee7285d4d 100644 --- a/tests/client/contexts/test_fib.py +++ b/tests/client/contexts/test_fib.py @@ -462,27 +462,38 @@ def test_file_transferred_to( "test_params", ( # Pass cases - (True, True, True, True, True, True, True, True, True), # DC images - (True, True, True, True, True, True, True, True, False), # No DC images + (True, True, True, True, True, True, True, True, True, True), # DC images + (True, True, True, True, True, True, True, True, True, False), # No DC images # Only one of these, and the last one, should be False at a given time - (True, True, True, True, True, True, True, False, False), # No activity name - (True, True, True, True, True, True, False, True, False), # No activity - (True, True, True, True, True, False, True, True, False), # No recipe name - (True, True, True, True, False, True, True, True, False), # No recipe - (True, True, True, False, True, True, True, True, False), # No site name - (True, True, False, True, True, True, True, True, False), # No sites - (True, False, True, True, True, True, True, True, False), # No project name - (False, True, True, True, True, True, True, True, False), # No environment + # No activity name + (True, True, True, True, True, True, True, True, False, False), + # No activity content + (True, True, True, True, True, True, True, False, True, False), + # No recipe name + (True, True, True, True, True, True, False, True, True, False), + # No recipe content + (True, True, True, True, True, False, True, True, True, False), + # No site name + (True, True, True, True, False, True, True, True, True, False), + # No site contents + (True, True, True, False, True, True, True, True, True, False), + # No project name + (True, True, False, True, True, True, True, True, True, False), + # No Murfey environment + (True, False, True, True, True, True, True, True, True, False), + # Not a target project + (False, True, True, True, True, True, True, True, True, False), ), ) def test_fib_full_autotem_context_projectdata( mocker: MockerFixture, - test_params: tuple[bool, bool, bool, bool, bool, bool, bool, bool, bool], + test_params: tuple[bool, bool, bool, bool, bool, bool, bool, bool, bool, bool], tmp_path: Path, visit_dir: Path, ): # Unpack test params ( + is_target_project, has_environment, has_project_name, has_sites, @@ -558,6 +569,8 @@ def test_fib_full_autotem_context_projectdata( machine_config={}, token="", ) + if is_target_project: + context._target_projects.append(project_name) if has_drift_correction_images: # Add drift correction images for i in range(num_lamellae): @@ -570,9 +583,14 @@ def test_fib_full_autotem_context_projectdata( # Run 'post_transfer' and check for expected calls and outputs context.post_transfer(mock_projectdata, environment=mock_environment) + # Check that the metadata file was added to the dictionary + if has_environment: + assert context._project_data[project_name] == mock_projectdata + # Check the success case if all( ( + is_target_project, has_environment, has_project_name, has_sites, @@ -623,6 +641,12 @@ def test_fib_full_autotem_context_projectdata( }, session_id=mock.ANY, ) + + # If the project name is not in the list, 'capture_post' won't be called + if not is_target_project: + mock_capture_post.assert_not_called() + + # These test parameters are related, with one being False at a time # These fail cases will return an empty dict and not call "post_transfer" if not has_environment: mock_logger.warning.assert_called_with("No environment passed in") From 6b339228ec37789861958ea0c9e8fc15dc54a894 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Wed, 1 Jul 2026 17:55:12 +0100 Subject: [PATCH 04/10] Migrated the lamella site info handling logic out into another class function so to make main 'post_transfer' function easier to understand --- src/murfey/client/contexts/fib.py | 198 +++++++++++++++--------------- 1 file changed, 99 insertions(+), 99 deletions(-) diff --git a/src/murfey/client/contexts/fib.py b/src/murfey/client/contexts/fib.py index ace16e9f6..5b02c7faf 100644 --- a/src/murfey/client/contexts/fib.py +++ b/src/murfey/client/contexts/fib.py @@ -289,113 +289,29 @@ def post_transfer( "/Sites/Lamella", ) ): + # Early exit if the file is not from a relevant project return None + # Mark project folder for analysis self._target_projects.append(project_name) logger.info( f"AutoTEM project {project_name!r} identified for registration" ) - # Analyse file and trigger processing if it passes the check + # Analyse file and trigger processing only from target projects if project_name in self._target_projects: - # Extract metadata from valid AutoTEM projects - if transferred_file.name == "ProjectData.dat" or ( - self._project_data.get(project_name) and not self._site_info - ): - # Extract directly from incoming file - if transferred_file.name == "ProjectData.dat": - logger.info( - f"Found metadata file {transferred_file} for parsing" - ) - all_site_info_new = self._parse_autotem_metadata( - transferred_file - ) - # Extract for the first time from stored file path - else: - project_data = self._project_data[project_name] - logger.info( - f"Performing initial metadata extraction from {project_data}" - ) - all_site_info_new = self._parse_autotem_metadata(project_data) - - # Parse the metadata file - for site_num, site_info_new in all_site_info_new.items(): - # Post the data to the backend if it's been changed - if ( - data := site_info_new.model_dump(exclude_none=True) - ) != self._site_info.get( - site_num, LamellaSiteInfo() - ).model_dump(exclude_none=True): - capture_post( - base_url=str(environment.url.geturl()), - router_name="workflow_fib.router", - function_name="register_fib_milling_progress", - token=self._token, - instrument_name=environment.instrument_name, - data=data, - # Endpoint kwargs - session_id=environment.murfey_session, - ) - - # Update existing dict - self._site_info[site_num] = site_info_new - logger.info(f"Updating metadata for site {site_num}") - - # Post drift correction GIF request if it hasn't already been done - fib_image = self._drift_correction_images.get(site_num, None) - if fib_image is not None and not fib_image.is_submitted: - # Construct the output file name if it doesn't already exist - if (output_file := fib_image.output_file) is None: - source = _get_source(transferred_file, environment) - if source is None: - logger.warning( - f"No source found for file {transferred_file}" - ) - continue - destination_file = _file_transferred_to( - environment=environment, - source=source, - file_path=transferred_file, - rsync_basepath=Path( - self._machine_config.get("rsync_basepath", "") - ), - ) - if destination_file is None: - logger.warning( - f"Could not find destination file path for {transferred_file.name!r}" - ) - continue - output_dir = self._determine_output_dir( - site_num, destination_file, environment - ) - if output_dir is None: - logger.warning( - f"Could not determine output directory for lamella {site_num}" - ) - continue - output_file = ( - output_dir - / "drift_correction" - / f"lamella_{site_num}.gif" - ) - with lock: - self._drift_correction_images[ - site_num - ].output_file = output_file - # Reload the new object - fib_image = self._drift_correction_images[site_num] - - if self._make_gif( - environment=environment, - lamella_number=site_num, - images=sorted(fib_image.images), - output_file=output_file, - ): - with lock: - self._drift_correction_images[ - site_num - ].is_submitted = True + # Perform first-time metadata extraction using stored "ProjectData.dat" file + if self._project_data.get(project_name) and not self._site_info: + project_data = self._project_data[project_name] + logger.info( + f"Performing initial metadata extraction from {project_data}" + ) + self._handle_autotem_metadata(project_data, environment) + # Extract metadata directly from "ProjectData.dat" on subsequent runs + if transferred_file.name == "ProjectData.dat" and self._site_info: + logger.info(f"Found metadata file {transferred_file} for parsing") + self._handle_autotem_metadata(transferred_file, environment) return None - + # Compile and register drift correction images elif ( "DCImages" in transferred_file.parts and transferred_file.suffix == ".png" @@ -595,6 +511,90 @@ def _determine_output_dir( ) return None + def _handle_autotem_metadata( + self, file: Path, environment: MurfeyInstanceEnvironment + ): + """ + Helper function to extract the AutoTEM metadata, update the stored FIB lamella + site info, and trigger relevant processing. + """ + + # Extract all site info + all_site_info_new = self._parse_autotem_metadata(file) + + # Parse the metadata file + for site_num, site_info_new in all_site_info_new.items(): + # Post the data to the backend if it's been changed + if ( + data := site_info_new.model_dump(exclude_none=True) + ) != self._site_info.get(site_num, LamellaSiteInfo()).model_dump( + exclude_none=True + ): + capture_post( + base_url=str(environment.url.geturl()), + router_name="workflow_fib.router", + function_name="register_fib_milling_progress", + token=self._token, + instrument_name=environment.instrument_name, + data=data, + # Endpoint kwargs + session_id=environment.murfey_session, + ) + + # Update existing dict + self._site_info[site_num] = site_info_new + logger.info(f"Updating metadata for site {site_num}") + + # Post drift correction GIF request if it hasn't already been done + fib_image = self._drift_correction_images.get(site_num, None) + if fib_image is not None and not fib_image.is_submitted: + # Construct the output file name if it doesn't already exist + if (output_file := fib_image.output_file) is None: + source = _get_source(file, environment) + if source is None: + logger.warning(f"No source found for file {file}") + continue + destination_file = _file_transferred_to( + environment=environment, + source=source, + file_path=file, + rsync_basepath=Path( + self._machine_config.get("rsync_basepath", "") + ), + ) + if destination_file is None: + logger.warning( + f"Could not find destination file path for {file.name!r}" + ) + continue + output_dir = self._determine_output_dir( + site_num, destination_file, environment + ) + if output_dir is None: + logger.warning( + f"Could not determine output directory for lamella {site_num}" + ) + continue + output_file = ( + output_dir / "drift_correction" / f"lamella_{site_num}.gif" + ) + with lock: + self._drift_correction_images[ + site_num + ].output_file = output_file + # Reload the new object + fib_image = self._drift_correction_images[site_num] + + if self._make_gif( + environment=environment, + lamella_number=site_num, + images=sorted(fib_image.images), + output_file=output_file, + ): + with lock: + self._drift_correction_images[site_num].is_submitted = True + return None + def _make_drift_correction_gif( self, file: Path, From 2fa9a96c2851dd6e4385d46359393b450679a771 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Wed, 1 Jul 2026 18:10:01 +0100 Subject: [PATCH 05/10] Updated tests to test '_handle_autotem_metadata' class function directly --- tests/client/contexts/test_fib.py | 60 ++++++++----------------------- 1 file changed, 15 insertions(+), 45 deletions(-) diff --git a/tests/client/contexts/test_fib.py b/tests/client/contexts/test_fib.py index ee7285d4d..03c42ac8b 100644 --- a/tests/client/contexts/test_fib.py +++ b/tests/client/contexts/test_fib.py @@ -462,39 +462,26 @@ def test_file_transferred_to( "test_params", ( # Pass cases - (True, True, True, True, True, True, True, True, True, True), # DC images - (True, True, True, True, True, True, True, True, True, False), # No DC images + (True, True, True, True, True, True, True, True), # DC images + (True, True, True, True, True, True, True, False), # No DC images # Only one of these, and the last one, should be False at a given time - # No activity name - (True, True, True, True, True, True, True, True, False, False), - # No activity content - (True, True, True, True, True, True, True, False, True, False), - # No recipe name - (True, True, True, True, True, True, False, True, True, False), - # No recipe content - (True, True, True, True, True, False, True, True, True, False), - # No site name - (True, True, True, True, False, True, True, True, True, False), - # No site contents - (True, True, True, False, True, True, True, True, True, False), - # No project name - (True, True, False, True, True, True, True, True, True, False), - # No Murfey environment - (True, False, True, True, True, True, True, True, True, False), - # Not a target project - (False, True, True, True, True, True, True, True, True, False), + (True, True, True, True, True, True, False, False), # No activity name + (True, True, True, True, True, False, True, False), # No activity content + (True, True, True, True, False, True, True, False), # No recipe name + (True, True, True, False, True, True, True, False), # No recipe content + (True, True, False, True, True, True, True, False), # No site name + (True, False, True, True, True, True, True, False), # No site contents + (False, True, True, True, True, True, True, False), # No project name ), ) -def test_fib_full_autotem_context_projectdata( +def test_handle_autotem_metadata( mocker: MockerFixture, - test_params: tuple[bool, bool, bool, bool, bool, bool, bool, bool, bool, bool], + test_params: tuple[bool, bool, bool, bool, bool, bool, bool, bool], tmp_path: Path, visit_dir: Path, ): # Unpack test params ( - is_target_project, - has_environment, has_project_name, has_sites, has_site_name, @@ -506,10 +493,8 @@ def test_fib_full_autotem_context_projectdata( ) = test_params # Mock the environment - mock_environment = None - if has_environment: - mock_environment = MagicMock() - mock_environment.visit = visit_name + mock_environment = MagicMock() + mock_environment.visit = visit_name # Mock the logger to check that specific logs are called mock_logger = mocker.patch("murfey.client.contexts.fib.logger") @@ -569,8 +554,6 @@ def test_fib_full_autotem_context_projectdata( machine_config={}, token="", ) - if is_target_project: - context._target_projects.append(project_name) if has_drift_correction_images: # Add drift correction images for i in range(num_lamellae): @@ -581,17 +564,11 @@ def test_fib_full_autotem_context_projectdata( ) # Run 'post_transfer' and check for expected calls and outputs - context.post_transfer(mock_projectdata, environment=mock_environment) - - # Check that the metadata file was added to the dictionary - if has_environment: - assert context._project_data[project_name] == mock_projectdata + context._handle_autotem_metadata(mock_projectdata, environment=mock_environment) # Check the success case if all( ( - is_target_project, - has_environment, has_project_name, has_sites, has_site_name, @@ -642,16 +619,9 @@ def test_fib_full_autotem_context_projectdata( session_id=mock.ANY, ) - # If the project name is not in the list, 'capture_post' won't be called - if not is_target_project: - mock_capture_post.assert_not_called() - # These test parameters are related, with one being False at a time # These fail cases will return an empty dict and not call "post_transfer" - if not has_environment: - mock_logger.warning.assert_called_with("No environment passed in") - mock_capture_post.assert_not_called() - elif not has_project_name: + if not has_project_name: mock_logger.warning.assert_called_with("Metadata file has no project name") mock_capture_post.assert_not_called() elif not has_sites: From cc8bce36a9874864f161221d78b5e69376c937f7 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Wed, 1 Jul 2026 18:29:01 +0100 Subject: [PATCH 06/10] Test '_make_drift_correction_gif' class function directly --- tests/client/contexts/test_fib.py | 43 ++++++++++++++++--------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/tests/client/contexts/test_fib.py b/tests/client/contexts/test_fib.py index 03c42ac8b..6c018a37e 100644 --- a/tests/client/contexts/test_fib.py +++ b/tests/client/contexts/test_fib.py @@ -663,35 +663,32 @@ def test_handle_autotem_metadata( @pytest.mark.parametrize( "test_params", ( + # Successful case + (True, True, True, True, True, True), # Early exits - # No MurfeyInstanceEnvironment - (False, True, True, True, True, True, True), # No source - (True, False, True, True, True, True, True), + (False, True, True, True, True, True), # No destination - (True, True, False, True, True, True, True), + (True, False, True, True, True, True), # No site info - (True, True, True, False, True, True, True), + (True, True, False, True, True, True), # No project name - (True, True, True, True, False, True, True), + (True, True, True, False, True, True), # No stage position - (True, True, True, True, True, False, True), + (True, True, True, True, False, True), # No stage position values - (True, True, True, True, True, True, False), - # Successful case - (True, True, True, True, True, True, True), + (True, True, True, True, True, False), ), ) -def test_fib_full_autotem_context_drift_correction_images( +def test_make_drift_correction_gif( mocker: MockerFixture, - test_params: tuple[bool, bool, bool, bool, bool, bool, bool], + test_params: tuple[bool, bool, bool, bool, bool, bool], tmp_path: Path, visit_dir: Path, fib_autotem_dc_images: list[Path], ): # Unpack test params ( - use_env, find_source, find_dst, has_site_info, @@ -701,10 +698,8 @@ def test_fib_full_autotem_context_drift_correction_images( ) = test_params # Mock the environment - mock_environment = None - if use_env: - mock_environment = MagicMock() - mock_environment.visit = visit_name + mock_environment = MagicMock() + mock_environment.visit = visit_name # Mock the logger to check if specific logs are triggered mock_logger = mocker.patch("murfey.client.contexts.fib.logger") @@ -757,31 +752,37 @@ def test_fib_full_autotem_context_drift_correction_images( # Parse images one-by-one and check that expected calls were made for file in fib_autotem_dc_images: - context.post_transfer(file, environment=mock_environment) - if not use_env: - mock_logger.warning.assert_called_with("No environment passed in") - elif not find_source: + context._make_drift_correction_gif(file, mock_environment) + + # For the fail cases, check that the correct log was called + if not find_source: mock_logger.warning.assert_called_with(f"No source found for file {file}") + mock_capture_post.assert_not_called() elif not find_dst: mock_logger.warning.assert_called_with( f"Could not find destination file path for {file.name!r}" ) + mock_capture_post.assert_not_called() elif not has_site_info: mock_logger.debug.assert_called_with( f"No metadata found for site {lamella_num} yet" ) + mock_capture_post.assert_not_called() elif not has_project_name: mock_logger.warning.assert_any_call( f"No project name associated with site {lamella_num}" ) + mock_capture_post.assert_not_called() elif not has_stage_position: mock_logger.warning.assert_any_call( f"No stage position information associated with site {lamella_num}" ) + mock_capture_post.assert_not_called() elif not has_stage_values: mock_logger.warning.assert_any_call( f"Could not determine slot number of site {lamella_num}" ) + mock_capture_post.assert_not_called() else: mock_get_source.assert_called_with(file, mock_environment) mock_file_transferred_to.assert_called_with( From de2d9864c47fbf5b65b16a06449c8e36a1c58108 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Thu, 2 Jul 2026 15:26:33 +0100 Subject: [PATCH 07/10] Update the '_make_drift_correction_gif' class function to post the correct request whether it is passed a client-side file path or the destination-side file path --- src/murfey/client/contexts/fib.py | 123 ++++++++++++------------------ 1 file changed, 50 insertions(+), 73 deletions(-) diff --git a/src/murfey/client/contexts/fib.py b/src/murfey/client/contexts/fib.py index 5b02c7faf..c24e11142 100644 --- a/src/murfey/client/contexts/fib.py +++ b/src/murfey/client/contexts/fib.py @@ -548,61 +548,22 @@ def _handle_autotem_metadata( # Post drift correction GIF request if it hasn't already been done fib_image = self._drift_correction_images.get(site_num, None) if fib_image is not None and not fib_image.is_submitted: - # Construct the output file name if it doesn't already exist - if (output_file := fib_image.output_file) is None: - source = _get_source(file, environment) - if source is None: - logger.warning(f"No source found for file {file}") - continue - destination_file = _file_transferred_to( - environment=environment, - source=source, - file_path=file, - rsync_basepath=Path( - self._machine_config.get("rsync_basepath", "") - ), - ) - if destination_file is None: - logger.warning( - f"Could not find destination file path for {file.name!r}" - ) - continue - output_dir = self._determine_output_dir( - site_num, destination_file, environment - ) - if output_dir is None: - logger.warning( - f"Could not determine output directory for lamella {site_num}" - ) - continue - output_file = ( - output_dir / "drift_correction" / f"lamella_{site_num}.gif" - ) - with lock: - self._drift_correction_images[ - site_num - ].output_file = output_file - # Reload the new object - fib_image = self._drift_correction_images[site_num] - - if self._make_gif( - environment=environment, - lamella_number=site_num, - images=sorted(fib_image.images), - output_file=output_file, - ): - with lock: - self._drift_correction_images[site_num].is_submitted = True + self._make_drift_correction_gif( + fib_image.images[-1], + environment, + is_destination_file=True, + ) return None def _make_drift_correction_gif( self, file: Path, environment: MurfeyInstanceEnvironment, + is_destination_file: bool = False, ): """ Helper function to create GIFs using the drift correction images seen by the - FIBContext class. The function uses the metadata returned + FIBContext class. The function uses the metadata extracted from the """ parts = file.parts try: @@ -613,19 +574,26 @@ def _make_drift_correction_gif( f"Could not extract metadata from file {file}", exc_info=True ) return None - source = _get_source(file, environment) - if source is None: - logger.warning(f"No source found for file {file}") - return - destination_file = _file_transferred_to( - environment=environment, - source=source, - file_path=file, - rsync_basepath=Path(self._machine_config.get("rsync_basepath", "")), - ) - if destination_file is None: - logger.warning(f"Could not find destination file path for {file.name!r}") - return + + # If the file provided is client-side, construct the destination file path + if not is_destination_file: + source = _get_source(file, environment) + if source is None: + logger.warning(f"No source found for file {file}") + return + destination_file = _file_transferred_to( + environment=environment, + source=source, + file_path=file, + rsync_basepath=Path(self._machine_config.get("rsync_basepath", "")), + ) + if destination_file is None: + logger.warning( + f"Could not find destination file path for {file.name!r}" + ) + return + else: + destination_file = file # Create FIBImage instance for this lamella site, or update existing one if not self._drift_correction_images.get(lamella_number): @@ -633,26 +601,35 @@ def _make_drift_correction_gif( self._drift_correction_images[lamella_number] = FIBImage( images=[destination_file] ) - else: + # Only update list if the file is not already in it + elif ( + destination_file not in self._drift_correction_images[lamella_number].images + ): with lock: self._drift_correction_images[lamella_number].images.append( destination_file ) self._drift_correction_images[lamella_number].is_submitted = False - if ( - output_dir := self._determine_output_dir( - lamella_number, - destination_file, - environment, - ) - ) is None: - logger.warning( - f"Could not determine output directory for lamella {lamella_number}" + + # If output GIF file path has not already been determined, construct it + output_file = self._drift_correction_images[lamella_number].output_file + if output_file is None: + if ( + output_dir := self._determine_output_dir( + lamella_number, + destination_file, + environment, + ) + ) is None: + logger.warning( + f"Could not determine output directory for lamella {lamella_number}" + ) + return None + output_file = ( + output_dir / "drift_correction" / f"lamella_{lamella_number}.gif" ) - return None - output_file = output_dir / "drift_correction" / f"lamella_{lamella_number}.gif" - with lock: - self._drift_correction_images[lamella_number].output_file = output_file + with lock: + self._drift_correction_images[lamella_number].output_file = output_file # Submit job to backend to construct a GIF if self._make_gif( From 7cfac6299b5c4e35bc2957beadf051809fa08305 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Thu, 2 Jul 2026 15:29:04 +0100 Subject: [PATCH 08/10] Update tests to check that '_make_drift_correction_gif' posts the correct request whether it's passed a client-side or destination-side file path --- tests/client/contexts/test_fib.py | 263 ++++++++++++++++-------------- 1 file changed, 145 insertions(+), 118 deletions(-) diff --git a/tests/client/contexts/test_fib.py b/tests/client/contexts/test_fib.py index 6c018a37e..e659401ba 100644 --- a/tests/client/contexts/test_fib.py +++ b/tests/client/contexts/test_fib.py @@ -16,6 +16,7 @@ _get_source, _parse_boolean, ) +from murfey.util.fib import number_from_name from murfey.util.models import LamellaSiteInfo # Mock session values @@ -499,39 +500,14 @@ def test_handle_autotem_metadata( # Mock the logger to check that specific logs are called mock_logger = mocker.patch("murfey.client.contexts.fib.logger") - # Mock '_get_source' - mock_get_source = mocker.patch("murfey.client.contexts.fib._get_source") - mock_get_source.return_value = tmp_path - - # Mock '_file_transferred_to' - mock_file_transferred_to = mocker.patch( - "murfey.client.contexts.fib._file_transferred_to" - ) - mock_file_transferred_to.return_value = ( - tmp_path - / "fib" - / "data" - / "current_year" - / visit_name - / "autotem" - / project_name - / "ProjectData.dat" - ) - # Set the expected output directory to be derived from metadata - output_dir = ( - tmp_path - / "fib" - / "data" - / "current_year" - / visit_name - / "processed" - / project_name - / "grid_2" - ) - # Mock the functions used in 'post_transfer' mock_capture_post = mocker.patch("murfey.client.contexts.fib.capture_post") + # Mock the '_make_drift_correction_gif' class function + mock_drift_correction_gif = mocker.patch( + "murfey.client.contexts.fib.FIBContext._make_drift_correction_gif" + ) + # Create the mock metadata file to parse mock_projectdata = create_fib_autotem_project_data( visit_dir=visit_dir, @@ -578,11 +554,6 @@ def test_handle_autotem_metadata( has_activity_name, ) ): - # 'capture_post' should be called once when registering the site - # and again if registering a drift correction image - assert mock_capture_post.call_count == num_lamellae * ( - 2 if has_drift_correction_images else 1 - ) # There should be one dictionary entry for each lamella now assert len(context._site_info) == num_lamellae for i in range(num_lamellae): @@ -599,24 +570,12 @@ def test_handle_autotem_metadata( mock_logger.info.assert_any_call( f"Updating metadata for site {lamella_number}" ) - + # If drift correction images are present, it should also call the next function if has_drift_correction_images: - mock_capture_post.assert_any_call( - base_url=mock.ANY, - router_name="workflow_fib.router", - function_name="make_gif", - token=mock.ANY, - instrument_name=mock.ANY, - data={ - "lamella_number": lamella_number, - "images": [str(tmp_path / "dummy.png")], - "output_file": str( - output_dir - / "drift_correction" - / f"lamella_{lamella_number}.gif" - ), - }, - session_id=mock.ANY, + mock_drift_correction_gif.assert_any_call( + context._drift_correction_images[lamella_number].images[-1], + mock_environment, + is_destination_file=True, ) # These test parameters are related, with one being False at a time @@ -664,25 +623,20 @@ def test_handle_autotem_metadata( "test_params", ( # Successful case - (True, True, True, True, True, True), + (True, True, True, True, True, True, True), # Using destination file path + (True, True, True, True, True, True, False), # Using client-side file path # Early exits - # No source - (False, True, True, True, True, True), - # No destination - (True, False, True, True, True, True), - # No site info - (True, True, False, True, True, True), - # No project name - (True, True, True, False, True, True), - # No stage position - (True, True, True, True, False, True), - # No stage position values - (True, True, True, True, True, False), + (False, True, True, True, True, True, False), # No source + (True, False, True, True, True, True, False), # No destination + (True, True, False, True, True, True, False), # No site info + (True, True, True, False, True, True, False), # No project name + (True, True, True, True, False, True, False), # No stage position + (True, True, True, True, True, False, False), # No stage position values ), ) def test_make_drift_correction_gif( mocker: MockerFixture, - test_params: tuple[bool, bool, bool, bool, bool, bool], + test_params: tuple[bool, bool, bool, bool, bool, bool, bool], tmp_path: Path, visit_dir: Path, fib_autotem_dc_images: list[Path], @@ -695,11 +649,15 @@ def test_make_drift_correction_gif( has_project_name, has_stage_position, has_stage_values, + use_destination_file, ) = test_params # Mock the environment mock_environment = MagicMock() mock_environment.visit = visit_name + mock_environment.url.geturl.return_value = "dummy" + mock_environment.instrument_name = "dummy" + mock_environment.murfey_session = 1 # Mock the logger to check if specific logs are triggered mock_logger = mocker.patch("murfey.client.contexts.fib.logger") @@ -750,55 +708,22 @@ def test_make_drift_correction_gif( if has_site_info: context._site_info[lamella_num] = LamellaSiteInfo(**metadata_dict) - # Parse images one-by-one and check that expected calls were made - for file in fib_autotem_dc_images: - context._make_drift_correction_gif(file, mock_environment) - - # For the fail cases, check that the correct log was called - if not find_source: - mock_logger.warning.assert_called_with(f"No source found for file {file}") - mock_capture_post.assert_not_called() - elif not find_dst: - mock_logger.warning.assert_called_with( - f"Could not find destination file path for {file.name!r}" - ) - mock_capture_post.assert_not_called() - elif not has_site_info: - mock_logger.debug.assert_called_with( - f"No metadata found for site {lamella_num} yet" - ) - mock_capture_post.assert_not_called() - elif not has_project_name: - mock_logger.warning.assert_any_call( - f"No project name associated with site {lamella_num}" - ) - mock_capture_post.assert_not_called() - elif not has_stage_position: - mock_logger.warning.assert_any_call( - f"No stage position information associated with site {lamella_num}" - ) - mock_capture_post.assert_not_called() - elif not has_stage_values: - mock_logger.warning.assert_any_call( - f"Could not determine slot number of site {lamella_num}" - ) - mock_capture_post.assert_not_called() - else: - mock_get_source.assert_called_with(file, mock_environment) - mock_file_transferred_to.assert_called_with( - environment=mock_environment, - source=basepath, - file_path=file, - rsync_basepath=Path(""), - ) - assert len(context._drift_correction_images) == num_lamellae - - for i in range(num_lamellae): - lamella_num = i + 1 - # The '_site_info' attribute should now be populated - assert ( - context._site_info[lamella_num].stage_info.preparation_site.slot_number - == 2 + # Run test and checks differently depending on input file type + if use_destination_file: + for file in destination_files: + # Find which lamella site it corresponds to + lamella_dir = file.relative_to( + destination_dir / "autotem" / "visit" / "Sites" + ) + lamella_num = number_from_name(lamella_dir.parts[0]) + + # Store destination file under corresponding site + if not context._drift_correction_images.get(lamella_num): + context._drift_correction_images[lamella_num] = FIBImage(images=[file]) + else: + context._drift_correction_images[lamella_num].images.append(file) + context._make_drift_correction_gif( + file, mock_environment, is_destination_file=True ) # The output file should point to 'grid_2' for a positive x stage position @@ -814,11 +739,113 @@ def test_make_drift_correction_gif( / "drift_correction" / f"lamella_{lamella_num}.gif" ) - assert ( - context._drift_correction_images[lamella_num].output_file == output_file + + # Check it made its way through to 'capture_post' + mock_capture_post.assert_any_call( + base_url="dummy", + router_name="workflow_fib.router", + function_name="make_gif", + token=context._token, + instrument_name="dummy", + data={ + "lamella_number": lamella_num, + "images": [ + str(file) + for file in context._drift_correction_images[lamella_num].images + ], + "output_file": str(output_file), + }, + session_id=1, ) - # 'capture_post' should be called for every image + + # 'capture_post' should have been called for each file assert mock_capture_post.call_count == len(destination_files) + else: + # Parse images one-by-one and check that expected calls were made + for file in fib_autotem_dc_images: + context._make_drift_correction_gif(file, mock_environment) + + # For the fail cases, check that the correct log was called + if not find_source: + mock_logger.warning.assert_called_with(f"No source found for file {file}") + mock_capture_post.assert_not_called() + elif not find_dst: + mock_logger.warning.assert_called_with( + f"Could not find destination file path for {file.name!r}" + ) + mock_capture_post.assert_not_called() + elif not has_site_info: + mock_logger.debug.assert_called_with( + f"No metadata found for site {lamella_num} yet" + ) + mock_capture_post.assert_not_called() + elif not has_project_name: + mock_logger.warning.assert_any_call( + f"No project name associated with site {lamella_num}" + ) + mock_capture_post.assert_not_called() + elif not has_stage_position: + mock_logger.warning.assert_any_call( + f"No stage position information associated with site {lamella_num}" + ) + mock_capture_post.assert_not_called() + elif not has_stage_values: + mock_logger.warning.assert_any_call( + f"Could not determine slot number of site {lamella_num}" + ) + mock_capture_post.assert_not_called() + else: + mock_get_source.assert_called_with(file, mock_environment) + mock_file_transferred_to.assert_called_with( + environment=mock_environment, + source=basepath, + file_path=file, + rsync_basepath=Path(""), + ) + assert len(context._drift_correction_images) == num_lamellae + + for i in range(num_lamellae): + lamella_num = i + 1 + + # The output file should point to 'grid_2' for a positive x stage position + output_file = ( + tmp_path + / "fib" + / "data" + / "current_year" + / visit_name + / "processed" + / project_name + / "grid_2" + / "drift_correction" + / f"lamella_{lamella_num}.gif" + ) + + # Check it made its way through to 'capture_post' + mock_capture_post.assert_any_call( + base_url="dummy", + router_name="workflow_fib.router", + function_name="make_gif", + token=context._token, + instrument_name="dummy", + data={ + "lamella_number": lamella_num, + "images": [ + str(file) + for file in context._drift_correction_images[ + lamella_num + ].images + ], + "output_file": str(output_file), + }, + session_id=1, + ) + assert ( + context._drift_correction_images[lamella_num].output_file + == output_file + ) + # 'capture_post' should be called for every image + assert mock_capture_post.call_count == len(destination_files) def test_fib_manual_autotem_context_projectdata( From c5cdcbf5f3f1d3fc3df9d035c0594dc291848bd6 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Thu, 2 Jul 2026 18:59:18 +0100 Subject: [PATCH 09/10] Update FIBContext test to check that file processing is triggered correctly when given files from multiple FIB projects --- tests/client/contexts/test_fib.py | 80 ++++++++++++++++++++++++------- 1 file changed, 64 insertions(+), 16 deletions(-) diff --git a/tests/client/contexts/test_fib.py b/tests/client/contexts/test_fib.py index e659401ba..dc6bb9905 100644 --- a/tests/client/contexts/test_fib.py +++ b/tests/client/contexts/test_fib.py @@ -13,6 +13,7 @@ FIBContext, FIBImage, _file_transferred_to, + _get_project_name, _get_source, _parse_boolean, ) @@ -848,26 +849,60 @@ def test_make_drift_correction_gif( assert mock_capture_post.call_count == len(destination_files) -def test_fib_manual_autotem_context_projectdata( +@pytest.mark.parametrize( + "test_params", + ( # Manual or automated? | Identifier + (True, "Sites/Site #1/DCImages/dummy.png"), + (True, "Sites/Site #1/LamellaEvaluationImages/dummy.png"), + (False, "Sites/Lamella/DCImages/dummy.png"), + ), +) +def test_fib_autotem_context( mocker: MockerFixture, visit_dir: Path, + test_params: tuple[bool, str], ): - # Mock the ProjectData.dat file - mock_projectdata = create_fib_autotem_project_data( - visit_dir=visit_dir, - project_name=f"AutoTEM_200101-1200_{project_name}", - site_prefix="Site", - ) + # Unpack test params + is_manual, trigger = test_params + + # Create list of files to pass through to FIBContext + if is_manual: + projects = [ + visit_dir / path + for path in ( # ProjectData.dat will be recorded and stored + f"autotem/AutoTEM_201231-1230_{visit_name}_waffle1/ProjectData.dat", + f"autotem/AutoTEM_201231-1230_{visit_name}_waffle2/ProjectData.dat", + f"autotem/AutoTEM_201231-1230_{visit_name}_waffle3/ProjectData.dat", + f"autotem/AutoTEM_201231-1230_{visit_name}_waffle4/ProjectData.dat", + ) + ] + target_project = ( + visit_dir + / f"autotem/AutoTEM_201231-1230_{visit_name}_waffle5/ProjectData.dat" + ) + trigger_file = ( + visit_dir / f"autotem/AutoTEM_201231-1230_{visit_name}_waffle5/{trigger}" + ) + else: + projects = [] + target_project = visit_dir / f"autotem/{visit_name}/ProjectData.dat" + trigger_file = visit_dir / f"autotem/{visit_name}/{trigger}" + files_to_pass: list[Path] = [ + *projects, + target_project, + trigger_file, + *projects, + target_project, + ] # Mock the Murfey environment mock_environment = MagicMock() - mock_environment.visit = visit_name - - # Patch the '_parse_autotem_metadata' class function - mock_parse = mocker.patch.object(FIBContext, "_parse_autotem_metadata") # Mock the functions used in 'post_transfer' - mock_capture_post = mocker.patch("murfey.client.contexts.fib.capture_post") + mock_handle_metadata = mocker.patch.object(FIBContext, "_handle_autotem_metadata") + mock_drift_correction_gif = mocker.patch.object( + FIBContext, "_make_drift_correction_gif" + ) # Initialise the FIBContext basepath = visit_dir @@ -878,10 +913,23 @@ def test_fib_manual_autotem_context_projectdata( token="", ) - # Pass file to FIBContext and check that it behaves as expected - context.post_transfer(mock_projectdata, environment=mock_environment) - mock_parse.assert_not_called() - mock_capture_post.assert_not_called() + # Pass files to FIBContext and check that it behaves as expected + for file in files_to_pass: + if not file.exists(): + file.parent.mkdir(parents=True) + file.touch() + context.post_transfer(file, environment=mock_environment) + # If a DCImage was used, '_make_drift_correction_gif' should be called + if "DCImages" in file.parts: + mock_drift_correction_gif.assert_called_with(file, mock_environment) + # All the ProjectData files should have been noted + assert len(context._project_data) == len(projects) + 1 + + # Target project will have been identified + assert _get_project_name(target_project) in context._target_projects + + # '_handle_metadata' will have been called + mock_handle_metadata.assert_called_with(target_project, mock_environment) def test_fib_maps_context( From 3d06e22b5daf1263d4cdd2b67931a9217780f649 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Thu, 2 Jul 2026 19:10:43 +0100 Subject: [PATCH 10/10] Update test logic --- tests/client/contexts/test_fib.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/client/contexts/test_fib.py b/tests/client/contexts/test_fib.py index dc6bb9905..6c469457a 100644 --- a/tests/client/contexts/test_fib.py +++ b/tests/client/contexts/test_fib.py @@ -887,13 +887,6 @@ def test_fib_autotem_context( projects = [] target_project = visit_dir / f"autotem/{visit_name}/ProjectData.dat" trigger_file = visit_dir / f"autotem/{visit_name}/{trigger}" - files_to_pass: list[Path] = [ - *projects, - target_project, - trigger_file, - *projects, - target_project, - ] # Mock the Murfey environment mock_environment = MagicMock() @@ -914,23 +907,30 @@ def test_fib_autotem_context( ) # Pass files to FIBContext and check that it behaves as expected - for file in files_to_pass: + for file in [*projects, target_project]: if not file.exists(): file.parent.mkdir(parents=True) file.touch() context.post_transfer(file, environment=mock_environment) - # If a DCImage was used, '_make_drift_correction_gif' should be called - if "DCImages" in file.parts: - mock_drift_correction_gif.assert_called_with(file, mock_environment) # All the ProjectData files should have been noted assert len(context._project_data) == len(projects) + 1 + # Pass the trigger file in + context.post_transfer(trigger_file, mock_environment) + # If a DCImage was used, '_make_drift_correction_gif' should be called + if "DCImages" in trigger_file.parts: + mock_drift_correction_gif.assert_called_with(trigger_file, mock_environment) # Target project will have been identified assert _get_project_name(target_project) in context._target_projects - # '_handle_metadata' will have been called mock_handle_metadata.assert_called_with(target_project, mock_environment) + # Create a dummy 'site_info' entry and parse "ProjectData.dat" + context._site_info[1] = LamellaSiteInfo() + context.post_transfer(target_project, mock_environment) + # '_handle_metadata' should now be called normally + mock_handle_metadata.assert_called_with(target_project, mock_environment) + def test_fib_maps_context( mocker: MockerFixture,