-
Notifications
You must be signed in to change notification settings - Fork 47
feat: Open Chamber fluidics support and FluidicsWidget enhancements #489
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Open Chamber fluidics support and FluidicsWidget enhancements #489
Conversation
- Add Open Chamber sequence names to validation: "Add Reagent", "Clear Tubings and Add Reagent", "Wash with Constant Flow" - Restrict "Flow " prefix sequences to MERFISH only - Hide manual control panel for Open Chamber application in FluidicsWidget Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Make sequence table editable for numeric columns (flow_rate, volume, incubation_time, repeat) - port columns remain read-only - Disable table editing when acquisition is in progress - Skip editing for Imaging rows - Add "Save Log" button to export status log to file - Stop fluidics operations when Stop Acquisition is pressed in MultiPointWithFluidicsWidget Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add abort check after fluidics wait_for_completion() to prevent imaging the first FOV when user stops acquisition during fluidics steps. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds Open Chamber application support to the fluidics system and enhances the FluidicsWidget with editing capabilities and log export functionality. It also improves user control by stopping fluidics operations when acquisition is aborted and prevents imaging when abort is requested during fluidics steps.
Changes:
- Added three new sequence names for Open Chamber mode: "Add Reagent", "Clear Tubings and Add Reagent", and "Wash with Constant Flow"
- Made numeric columns in the FluidicsWidget sequence table editable (flow_rate, volume, incubation_time, repeat)
- Added "Save Log" button to export fluidics status logs and emergency stop functionality when stopping acquisition
- Conditionally hide manual control panel for Open Chamber mode while keeping it visible for MERFISH
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| software/control/widgets.py | Added Save Log button, made sequence table editable, conditionally display manual controls, and stop fluidics on acquisition abort |
| software/control/fluidics.py | Extended sequence validation to support Open Chamber-specific sequence names |
| software/control/core/multi_point_worker.py | Added abort check after fluidics completion to skip imaging if user stopped during fluidics |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| manual_control_group.setLayout(manual_control_layout) | ||
| left_panel.addWidget(manual_control_group) | ||
| # Manual Control panel (MERFISH only, not for Open Chamber) | ||
| self.is_open_chamber = self.fluidics.config.get("application") == "Open Chamber" |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Open Chamber application mode check is duplicated across multiple methods. Consider storing this as a class attribute in init or as a property to avoid repeated dictionary lookups and improve maintainability.
| self.is_open_chamber = self.fluidics.config.get("application") == "Open Chamber" | |
| if not hasattr(self, "is_open_chamber"): | |
| self.is_open_chamber = self.fluidics.config.get("application") == "Open Chamber" |
software/control/widgets.py
Outdated
| # For EditRole, return raw value for editing | ||
| if role == Qt.EditRole: | ||
| if column_name in self._editable_columns: | ||
| return int(value) if not pd.isna(value) else 0 |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returning 0 for missing values in EditRole could be confusing and may lead to unintended edits. Consider returning None or an empty string to better represent missing data, or add validation to prevent editing cells with missing values.
| return int(value) if not pd.isna(value) else 0 | |
| return int(value) if not pd.isna(value) else None |
software/control/widgets.py
Outdated
| if column_name not in self._editable_columns: | ||
| return False | ||
| try: | ||
| self._data.iloc[index.row(), index.column()] = int(value) |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The setData method only converts values to int, but the editable columns may include float values (e.g., flow_rate could be 0.5 mL/min). This will truncate decimal values. Consider using float() conversion or add type-specific handling based on the column.
| self._data.iloc[index.row(), index.column()] = int(value) | |
| if column_name == "repeat": | |
| converted_value = int(value) | |
| else: | |
| converted_value = float(value) | |
| self._data.iloc[index.row(), index.column()] = converted_value |
| for idx, name in enumerate(self.sequences["sequence_name"]): | ||
| is_valid = name in valid_names or (not is_open_chamber and name.startswith("Flow ")) | ||
| if not is_valid: | ||
| hint = f"Must be one of {valid_names}" + ("" if is_open_chamber else " or start with 'Flow '") |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String concatenation with conditional suffix is harder to read. Consider using an f-string with conditional expression or separate the hint construction into clearer conditional blocks.
| hint = f"Must be one of {valid_names}" + ("" if is_open_chamber else " or start with 'Flow '") | |
| hint = ( | |
| f"Must be one of {valid_names}" | |
| if is_open_chamber | |
| else f"Must be one of {valid_names} or start with 'Flow '" | |
| ) |
- Use fluidics.sequences directly instead of maintaining a copy - Hide "include" column in view instead of dropping it - Add input validation for editable numeric fields - Make emergency_stop() defensive (check for None) - Exclude ndviewer_light from black formatting Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Extract _validate_int_field helper for DRY validation - Use class selection pattern in _initialize_hardware - Extract _get_port_name and _is_valid_column_value helpers - Replace bare except with explicit None check in on_finish - Flatten nested conditionals in PandasTableModel.data Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add set_acquisition_running() slot to FluidicsWidget - Connect signal_acquisition_started to disable editing when running Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Tested in simulation mode
Test plan
🤖 Generated with Claude Code