Allow manual editing of registration result, including for time lapses#20
Conversation
…or time lapses. Also, registration runs will take into account currently visible parameters (both original and registered) without the need to reload layers
There was a problem hiding this comment.
Pull request overview
This PR adds support for manually editing transforms used for stitching/registration (including time-lapse scenarios), and ensures subsequent registrations/fusions use the currently visible/adjusted parameters without requiring layer reloads.
Changes:
- Add UI controls to choose the registration method (Phase Correlation vs ITKElastix) and configure transform types.
- Capture manual pre-registration layer transforms into
affine_metadataand live-updateaffine_registeredfor the current timepoint when showing “Registered”. - Add tests covering manual transform capture pre-registration and post-registration (timepoint-specific updates).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/napari_stitcher/_stitcher_widget.py |
Adds method selection UI and implements transform capture/live update logic for manual edits and timepoint scrolling. |
src/napari_stitcher/_tests/test_stitcher_widget.py |
Adds tests validating manual transform persistence in affine_metadata / affine_registered (including per-timepoint behavior). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # - only proceed after registration has been performed | ||
| # - only proceed when the timepoint actually changed (transform-mode | ||
| # interactions also fire current_step without changing the tp) | ||
| if hasattr(event, 'type'): | ||
| if not self.visualization_type_rbuttons.enabled: | ||
| return |
There was a problem hiding this comment.
update_viewer_transformations returns early on current_step events when visualization_type_rbuttons is disabled. Since that control is only enabled after registration, scrolling timepoints after loading layers (but before registration) will no longer apply per-timepoint affine_metadata transforms, which breaks time-lapse stage-shift visualization and can cause registration to start from the wrong (tp=0) transforms. Consider allowing CHOICE_METADATA updates regardless of the radio-buttons' enabled state (or gating only the Registered path).
| # - only proceed after registration has been performed | |
| # - only proceed when the timepoint actually changed (transform-mode | |
| # interactions also fire current_step without changing the tp) | |
| if hasattr(event, 'type'): | |
| if not self.visualization_type_rbuttons.enabled: | |
| return | |
| # - allow metadata-based per-timepoint updates even before | |
| # registration has been performed | |
| # - only proceed when the timepoint actually changed (transform-mode | |
| # interactions also fire current_step without changing the tp) | |
| if hasattr(event, 'type'): |
| t_coords = sim.coords['t'] if 't' in sim.dims else None | ||
| affine_xr = param_utils.affine_to_xaffine(affine, t_coords=t_coords) |
There was a problem hiding this comment.
_capture_layer_transforms_to_msims builds an xarray affine using t_coords and a single matrix from l.affine, which will broadcast the current affine to all timepoints. For time-lapse datasets with time-varying affine_metadata, this overwrites all frames and loses stage-shift history. Instead, when t is present, update only the current timepoint entry in the existing affine_metadata (similar to _update_registered_param_for_current_tp).
| t_coords = sim.coords['t'] if 't' in sim.dims else None | |
| affine_xr = param_utils.affine_to_xaffine(affine, t_coords=t_coords) | |
| if 't' in sim.dims: | |
| # Preserve existing per-timepoint affine_metadata and update only | |
| # the current timepoint, mirroring registered transform updates. | |
| sdims = spatial_image_utils.get_spatial_dims_from_sim(sim) | |
| if len(self.viewer.dims.current_step) > len(sdims): | |
| curr_tp = self.viewer.dims.current_step[-len(sdims) - 1] | |
| else: | |
| curr_tp = 0 | |
| try: | |
| affine_xr = spatial_image_utils.get_affine_from_sim( | |
| sim, transform_key='affine_metadata').copy() | |
| except Exception: | |
| affine_xr = param_utils.affine_to_xaffine( | |
| affine, t_coords=sim.coords['t']) | |
| if 't' in affine_xr.dims: | |
| t_val = sim.coords['t'][curr_tp] | |
| affine_xr.loc[{'t': t_val}] = affine | |
| else: | |
| affine_xr = param_utils.affine_to_xaffine( | |
| affine, t_coords=sim.coords['t']) | |
| t_val = sim.coords['t'][curr_tp] | |
| affine_xr.loc[{'t': t_val}] = affine | |
| else: | |
| affine_xr = param_utils.affine_to_xaffine(affine, t_coords=None) |
| self.antspy_transform_types = widgets.Select( | ||
| choices=['Translation', 'Rigid', 'Affine'], | ||
| value=['Translation', 'Rigid'], | ||
| label='Transform types:', | ||
| tooltip='Sequence of transform types applied in order. The last selected type is also used for global optimization.') |
There was a problem hiding this comment.
The widget is labeled for ITKElastix, but the variable name antspy_transform_types suggests ANTsPy. Renaming this to something like elastix_transform_types (and updating downstream uses) would avoid confusion and make the UI code easier to maintain.
| try: | ||
| params = spatial_image_utils.get_affine_from_sim( | ||
| sims[il], transform_key=transform_key | ||
| ) | ||
| except: |
There was a problem hiding this comment.
Avoid bare except: here: it can hide unexpected errors (including bugs inside get_affine_from_sim) and makes failures silent. Prefer except Exception as e and either re-raise in debug/test contexts or at least log/notify with the exception message so users can diagnose missing/invalid transforms.
| raise(Exception()) | ||
| except: | ||
| notifications.notification_manager.receive_info( | ||
| 'Timepoint %s: no parameters available, register first.' % curr_tp) | ||
| continue |
There was a problem hiding this comment.
This second bare except: also swallows all error types and can make diagnosing malformed parameters difficult. Consider catching specific exceptions for missing t coordinates / selection errors (e.g. KeyError, IndexError, ValueError) and use except Exception as e as a fallback that preserves the error details in the notification.
| raise(Exception()) | |
| except: | |
| notifications.notification_manager.receive_info( | |
| 'Timepoint %s: no parameters available, register first.' % curr_tp) | |
| continue | |
| raise ValueError("parameters contain NaN values") | |
| except (KeyError, IndexError, ValueError) as e: | |
| notifications.notification_manager.receive_info( | |
| 'Timepoint %s: no parameters available, register first. Details: %s' | |
| % (curr_tp, e)) | |
| continue | |
| except Exception as e: | |
| notifications.notification_manager.receive_info( | |
| 'Timepoint %s: unable to apply parameters. Details: %s' | |
| % (curr_tp, e)) | |
| continue |
Add possibility to correct parameters after registration, including for time lapses. Also, registration runs will take into account currently visible parameters (both original and registered) without the need to reload layers