feat: Add Views tab to Configuration dialog for downsampled view sett… #395
Conversation
…ings Add new Views tab to PreferencesDialog with settings for: - Generate Downsampled Well Images - Display Plate View - Well Resolutions (comma-separated) - Plate Resolution - Z-Projection Mode (mip/middle) - Mosaic Target Pixel Size Settings are saved to config file and applied to _def module at runtime. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a new "Views" tab to the Configuration/PreferencesDialog for managing downsampled view settings during acquisition. The settings control well and plate view generation, resolution parameters, z-projection modes, and mosaic view pixel sizes.
Key changes:
- New Views tab UI with 6 configurable settings for downsampled view generation
- Settings persistence to config file under new "VIEWS" section
- Runtime application of settings to
_defmodule variables
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| software/control/widgets.py | Adds _create_views_tab() method to create the new UI tab, updates _apply_settings() and _apply_live_settings() to persist and apply Views configuration |
| software/tests/control/test_preferences_dialog.py | Adds comprehensive test suite for Views tab including initialization tests, save-to-file tests, and runtime application tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try: | ||
| _def.DOWNSAMPLED_WELL_RESOLUTIONS_UM = [float(x.strip()) for x in resolutions_str.split(",") if x.strip()] | ||
| except ValueError: | ||
| self._log.warning(f"Invalid well resolutions format: {resolutions_str}") |
There was a problem hiding this comment.
When parsing the well resolutions fails due to invalid format, the exception is caught and logged but the original value in _def.DOWNSAMPLED_WELL_RESOLUTIONS_UM is not preserved. This means if the user enters invalid data, the value will remain as whatever it was before, potentially causing confusion. Consider either preserving the original value explicitly, or providing user feedback about the invalid input before allowing the settings to be applied.
| self.well_resolutions_edit = QLineEdit() | ||
| default_resolutions = self._get_config_value("VIEWS", "downsampled_well_resolutions_um", "5.0, 10.0, 20.0") | ||
| self.well_resolutions_edit.setText(default_resolutions) | ||
| self.well_resolutions_edit.setToolTip("Comma-separated list of resolution values in micrometers") | ||
| layout.addRow("Well Resolutions (μm):", self.well_resolutions_edit) |
There was a problem hiding this comment.
The well_resolutions_edit field accepts any text input without validation. Users could enter invalid formats (like non-numeric values, negative numbers, or empty strings) which would only be caught later when applying settings. Consider adding input validation using QRegExpValidator or a custom validator to ensure only valid comma-separated numeric values can be entered, providing immediate feedback to the user.
| def test_views_settings_applied_to_def(self, preferences_dialog): | ||
| import control._def as _def | ||
|
|
||
| preferences_dialog.generate_downsampled_checkbox.setChecked(True) | ||
| preferences_dialog.display_plate_view_checkbox.setChecked(True) | ||
| preferences_dialog.plate_resolution_spinbox.setValue(20.0) | ||
| preferences_dialog.mosaic_pixel_size_spinbox.setValue(4.0) | ||
|
|
||
| preferences_dialog._apply_live_settings() | ||
|
|
||
| assert _def.GENERATE_DOWNSAMPLED_WELL_IMAGES is True | ||
| assert _def.DISPLAY_PLATE_VIEW is True | ||
| assert _def.DOWNSAMPLED_PLATE_RESOLUTION_UM == 20.0 | ||
| assert _def.MOSAIC_VIEW_TARGET_PIXEL_SIZE_UM == 4.0 |
There was a problem hiding this comment.
The test for applying views settings to the _def module does not verify that well resolutions are correctly parsed and applied, nor does it test the z-projection enum conversion. Consider adding assertions to verify _def.DOWNSAMPLED_WELL_RESOLUTIONS_UM and _def.DOWNSAMPLED_Z_PROJECTION are set correctly, and add a test case for invalid well resolution format to ensure error handling works as expected.
…ings
Add new Views tab to PreferencesDialog with settings for:
Settings are saved to config file and applied to _def module at runtime.