Skip to content

feat: Add Views tab to Configuration dialog for downsampled view settings #395

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.

Sign up for GitHub

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

Merged
hongquanli merged 7 commits into from
Jan 6, 2026
+470 −53
Merged

feat: Add Views tab to Configuration dialog for downsampled view settings #395

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
afc1193
feat: Add Views tab to Configuration dialog for downsampled view sett…
hongquanli Dec 29, 2025
1c7f944
fix: Add input validation and change detection for Views settings
hongquanli Jan 6, 2026
a66ce03
feat: Improve CollapsibleGroupBox UI with arrow indicators and borders
hongquanli Jan 6, 2026
4d3f2d2
feat: Add Display Mosaic View setting and load Views config on startup
hongquanli Jan 6, 2026
24cb2f2
fix: Add error handling for ZProjectionMode.convert_to_enum
hongquanli Jan 6, 2026
5fe81e3
refactor: Revert control._def.XXX to direct variable access
hongquanli Jan 6, 2026
8864341
fix: Improve test naming and regex validation
hongquanli Jan 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions software/control/_def.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1045,3 +1045,51 @@ def get_wellplate_settings(wellplate_format):
# saving path
if not (DEFAULT_SAVING_PATH.startswith(str(Path.home()))):
DEFAULT_SAVING_PATH = str(Path.home()) + "/" + DEFAULT_SAVING_PATH.strip("/")

# Load Views settings from config file at startup
# These values override the defaults above and are accessed via control._def.XXX
if CACHED_CONFIG_FILE_PATH and os.path.exists(CACHED_CONFIG_FILE_PATH):
try:
_views_config = ConfigParser()
_views_config.read(CACHED_CONFIG_FILE_PATH)
if _views_config.has_section("VIEWS"):
log.info("Loading Views settings from config file")
if _views_config.has_option("VIEWS", "display_plate_view"):
DISPLAY_PLATE_VIEW = _views_config.get("VIEWS", "display_plate_view").lower() in ("true", "1", "yes")
if _views_config.has_option("VIEWS", "display_mosaic_view"):
USE_NAPARI_FOR_MOSAIC_DISPLAY = _views_config.get("VIEWS", "display_mosaic_view").lower() in (
"true",
"1",
"yes",
)
if _views_config.has_option("VIEWS", "generate_downsampled_well_images"):
GENERATE_DOWNSAMPLED_WELL_IMAGES = _views_config.get(
"VIEWS", "generate_downsampled_well_images"
).lower() in ("true", "1", "yes")
if _views_config.has_option("VIEWS", "downsampled_well_resolutions_um"):
try:
_res_str = _views_config.get("VIEWS", "downsampled_well_resolutions_um")
DOWNSAMPLED_WELL_RESOLUTIONS_UM = [float(x.strip()) for x in _res_str.split(",") if x.strip()]
except ValueError:
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'except' clause does nothing but pass and there is no explanatory comment.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@hongquanli hongquanli Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code] Skipped - intentional behavior: if parsing fails, keep the default value. The outer try/except (line 1091) logs any errors.

Copy link
Contributor Author

@hongquanli hongquanli Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@claude what's the default value?

pass
if _views_config.has_option("VIEWS", "downsampled_plate_resolution_um"):
try:
DOWNSAMPLED_PLATE_RESOLUTION_UM = _views_config.getfloat("VIEWS", "downsampled_plate_resolution_um")
except ValueError:
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'except' clause does nothing but pass and there is no explanatory comment.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@hongquanli hongquanli Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code] Skipped - same as above.

pass
if _views_config.has_option("VIEWS", "downsampled_z_projection"):
try:
DOWNSAMPLED_Z_PROJECTION = ZProjectionMode.convert_to_enum(
_views_config.get("VIEWS", "downsampled_z_projection")
)
except ValueError:
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'except' clause does nothing but pass and there is no explanatory comment.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@hongquanli hongquanli Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code] Skipped - repeat comment. The outer try/except at line 1091 already logs errors.

pass
if _views_config.has_option("VIEWS", "mosaic_view_target_pixel_size_um"):
try:
MOSAIC_VIEW_TARGET_PIXEL_SIZE_UM = _views_config.getfloat(
"VIEWS", "mosaic_view_target_pixel_size_um"
)
except ValueError:
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'except' clause does nothing but pass and there is no explanatory comment.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@hongquanli hongquanli Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code] Skipped - same as above.

pass
Comment on lines +1073 to +1093
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'except' clause does nothing but pass and there is no explanatory comment.

Suggested change
except ValueError:
pass
if _views_config.has_option("VIEWS", "downsampled_plate_resolution_um"):
try:
DOWNSAMPLED_PLATE_RESOLUTION_UM = _views_config.getfloat("VIEWS", "downsampled_plate_resolution_um")
except ValueError:
pass
if _views_config.has_option("VIEWS", "downsampled_z_projection"):
try:
DOWNSAMPLED_Z_PROJECTION = ZProjectionMode.convert_to_enum(
_views_config.get("VIEWS", "downsampled_z_projection")
)
except ValueError:
pass
if _views_config.has_option("VIEWS", "mosaic_view_target_pixel_size_um"):
try:
MOSAIC_VIEW_TARGET_PIXEL_SIZE_UM = _views_config.getfloat(
"VIEWS", "mosaic_view_target_pixel_size_um"
)
except ValueError:
pass
except ValueError as err:
log.warning(
"Invalid value for VIEWS.downsampled_well_resolutions_um in config; "
"keeping existing default. Error: %s",
err,
)
if _views_config.has_option("VIEWS", "downsampled_plate_resolution_um"):
try:
DOWNSAMPLED_PLATE_RESOLUTION_UM = _views_config.getfloat("VIEWS", "downsampled_plate_resolution_um")
except ValueError as err:
log.warning(
"Invalid value for VIEWS.downsampled_plate_resolution_um in config; "
"keeping existing default. Error: %s",
err,
)
if _views_config.has_option("VIEWS", "downsampled_z_projection"):
try:
DOWNSAMPLED_Z_PROJECTION = ZProjectionMode.convert_to_enum(
_views_config.get("VIEWS", "downsampled_z_projection")
)
except ValueError as err:
log.warning(
"Invalid value for VIEWS.downsampled_z_projection in config; "
"keeping existing default. Error: %s",
err,
)
if _views_config.has_option("VIEWS", "mosaic_view_target_pixel_size_um"):
try:
MOSAIC_VIEW_TARGET_PIXEL_SIZE_UM = _views_config.getfloat(
"VIEWS", "mosaic_view_target_pixel_size_um"
)
except ValueError as err:
log.warning(
"Invalid value for VIEWS.mosaic_view_target_pixel_size_um in config; "
"keeping existing default. Error: %s",
err,
)

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@hongquanli hongquanli Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code] Skipped - the outer try/except at line 1091 already logs errors with log.warning(). Adding per-field logging would be verbose.

Comment on lines +1073 to +1093
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'except' clause does nothing but pass and there is no explanatory comment.

Suggested change
except ValueError:
pass
if _views_config.has_option("VIEWS", "downsampled_plate_resolution_um"):
try:
DOWNSAMPLED_PLATE_RESOLUTION_UM = _views_config.getfloat("VIEWS", "downsampled_plate_resolution_um")
except ValueError:
pass
if _views_config.has_option("VIEWS", "downsampled_z_projection"):
try:
DOWNSAMPLED_Z_PROJECTION = ZProjectionMode.convert_to_enum(
_views_config.get("VIEWS", "downsampled_z_projection")
)
except ValueError:
pass
if _views_config.has_option("VIEWS", "mosaic_view_target_pixel_size_um"):
try:
MOSAIC_VIEW_TARGET_PIXEL_SIZE_UM = _views_config.getfloat(
"VIEWS", "mosaic_view_target_pixel_size_um"
)
except ValueError:
pass
except ValueError as exc:
log.warning(
"Invalid value for 'downsampled_well_resolutions_um' in [VIEWS] "
f"from config file {CACHED_CONFIG_FILE_PATH!r}: {exc}"
)
if _views_config.has_option("VIEWS", "downsampled_plate_resolution_um"):
try:
DOWNSAMPLED_PLATE_RESOLUTION_UM = _views_config.getfloat("VIEWS", "downsampled_plate_resolution_um")
except ValueError as exc:
log.warning(
"Invalid value for 'downsampled_plate_resolution_um' in [VIEWS] "
f"from config file {CACHED_CONFIG_FILE_PATH!r}: {exc}"
)
if _views_config.has_option("VIEWS", "downsampled_z_projection"):
try:
DOWNSAMPLED_Z_PROJECTION = ZProjectionMode.convert_to_enum(
_views_config.get("VIEWS", "downsampled_z_projection")
)
except ValueError as exc:
log.warning(
"Invalid value for 'downsampled_z_projection' in [VIEWS] "
f"from config file {CACHED_CONFIG_FILE_PATH!r}: {exc}"
)
if _views_config.has_option("VIEWS", "mosaic_view_target_pixel_size_um"):
try:
MOSAIC_VIEW_TARGET_PIXEL_SIZE_UM = _views_config.getfloat(
"VIEWS", "mosaic_view_target_pixel_size_um"
)
except ValueError as exc:
log.warning(
"Invalid value for 'mosaic_view_target_pixel_size_um' in [VIEWS] "
f"from config file {CACHED_CONFIG_FILE_PATH!r}: {exc}"
)

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@hongquanli hongquanli Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code] Skipped - same as above.

Comment on lines +1073 to +1093
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'except' clause does nothing but pass and there is no explanatory comment.

Suggested change
except ValueError:
pass
if _views_config.has_option("VIEWS", "downsampled_plate_resolution_um"):
try:
DOWNSAMPLED_PLATE_RESOLUTION_UM = _views_config.getfloat("VIEWS", "downsampled_plate_resolution_um")
except ValueError:
pass
if _views_config.has_option("VIEWS", "downsampled_z_projection"):
try:
DOWNSAMPLED_Z_PROJECTION = ZProjectionMode.convert_to_enum(
_views_config.get("VIEWS", "downsampled_z_projection")
)
except ValueError:
pass
if _views_config.has_option("VIEWS", "mosaic_view_target_pixel_size_um"):
try:
MOSAIC_VIEW_TARGET_PIXEL_SIZE_UM = _views_config.getfloat(
"VIEWS", "mosaic_view_target_pixel_size_um"
)
except ValueError:
pass
except ValueError as e:
log.warning(
"Invalid value for 'downsampled_well_resolutions_um' in VIEWS config "
f"({CACHED_CONFIG_FILE_PATH}): {_res_str!r} ({e}). Using default value(s)."
)
if _views_config.has_option("VIEWS", "downsampled_plate_resolution_um"):
try:
DOWNSAMPLED_PLATE_RESOLUTION_UM = _views_config.getfloat("VIEWS", "downsampled_plate_resolution_um")
except ValueError as e:
log.warning(
"Invalid float for 'downsampled_plate_resolution_um' in VIEWS config "
f"({CACHED_CONFIG_FILE_PATH}): {_views_config.get('VIEWS', 'downsampled_plate_resolution_um', fallback='')!r} ({e}). Using default value."
)
if _views_config.has_option("VIEWS", "downsampled_z_projection"):
try:
DOWNSAMPLED_Z_PROJECTION = ZProjectionMode.convert_to_enum(
_views_config.get("VIEWS", "downsampled_z_projection")
)
except ValueError as e:
log.warning(
"Invalid value for 'downsampled_z_projection' in VIEWS config "
f"({CACHED_CONFIG_FILE_PATH}). Using default value. Details: {e}"
)
if _views_config.has_option("VIEWS", "mosaic_view_target_pixel_size_um"):
try:
MOSAIC_VIEW_TARGET_PIXEL_SIZE_UM = _views_config.getfloat(
"VIEWS", "mosaic_view_target_pixel_size_um"
)
except ValueError as e:
log.warning(
"Invalid float for 'mosaic_view_target_pixel_size_um' in VIEWS config "
f"({CACHED_CONFIG_FILE_PATH}): "
f"{_views_config.get('VIEWS', 'mosaic_view_target_pixel_size_um', fallback='')!r} ({e}). Using default value."
)

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@hongquanli hongquanli Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code] Skipped - same as above.

except Exception as e:
log.warning(f"Failed to load Views settings from config: {e}")
58 changes: 29 additions & 29 deletions software/control/gui_hcs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -792,16 +792,18 @@ def setupImageDisplayTabs(self):
self.imageArrayDisplayWindow = core.ImageArrayDisplayWindow()
self.imageDisplayTabs.addTab(self.imageArrayDisplayWindow.widget, "Multichannel Acquisition")

self.napariMosaicDisplayWidget = None
if USE_NAPARI_FOR_MOSAIC_DISPLAY:
self.napariMosaicDisplayWidget = widgets.NapariMosaicDisplayWidget(
self.objectiveStore, self.camera, self.contrastManager
)
self.imageDisplayTabs.addTab(self.napariMosaicDisplayWidget, "Mosaic View")

# Plate view for well-based acquisitions (only if enabled)
if control._def.DISPLAY_PLATE_VIEW:
self.napariPlateViewWidget = widgets.NapariPlateViewWidget(self.contrastManager)
self.imageDisplayTabs.addTab(self.napariPlateViewWidget, "Plate View")
# Plate view for well-based acquisitions (independent of mosaic view)
self.napariPlateViewWidget = None
if DISPLAY_PLATE_VIEW:
self.napariPlateViewWidget = widgets.NapariPlateViewWidget(self.contrastManager)
self.imageDisplayTabs.addTab(self.napariPlateViewWidget, "Plate View")

# z plot
self.zPlotWidget = widgets.SurfacePlotWidget()
Expand Down Expand Up @@ -1333,23 +1335,23 @@ def makeNapariConnections(self):
]
)

# Setup plate view widget connections (only if plate view is enabled)
# Use Qt.QueuedConnection explicitly for thread safety since these signals
# are emitted from the acquisition worker thread and received on the main thread.
# This ensures the slot is invoked in the receiver's thread event loop.
if control._def.DISPLAY_PLATE_VIEW and hasattr(self, "napariPlateViewWidget"):
self.napari_connections["napariPlateViewWidget"] = [
(
self.multipointController.plate_view_init,
self.napariPlateViewWidget.initPlateLayout,
Qt.QueuedConnection,
),
(
self.multipointController.plate_view_update,
self.napariPlateViewWidget.updatePlateView,
Qt.QueuedConnection,
),
]
# Setup plate view widget connections (independent of mosaic display)
# Use Qt.QueuedConnection explicitly for thread safety since these signals
# are emitted from the acquisition worker thread and received on the main thread.
# This ensures the slot is invoked in the receiver's thread event loop.
if self.napariPlateViewWidget is not None:
self.napari_connections["napariPlateViewWidget"] = [
(
self.multipointController.plate_view_init,
self.napariPlateViewWidget.initPlateLayout,
Qt.QueuedConnection,
),
(
self.multipointController.plate_view_update,
self.napariPlateViewWidget.updatePlateView,
Qt.QueuedConnection,
),
]

# Make initial connections
self.updateNapariConnections()
Expand Down Expand Up @@ -1411,14 +1413,12 @@ def setAcquisitionDisplayTabs(self, selected_configurations, Nz, xy_mode=None):
elif not self.live_only_mode:
configs = [config.name for config in selected_configurations]
print(configs)
if USE_NAPARI_FOR_MOSAIC_DISPLAY and Nz == 1:
# For well-based acquisitions (Select Wells or Load Coordinates), use Plate View
is_well_based = xy_mode is not None and xy_mode in ("Select Wells", "Load Coordinates")
if is_well_based and hasattr(self, "napariPlateViewWidget") and control._def.DISPLAY_PLATE_VIEW:
self.imageDisplayTabs.setCurrentWidget(self.napariPlateViewWidget)
else:
self.imageDisplayTabs.setCurrentWidget(self.napariMosaicDisplayWidget)

# For well-based acquisitions (Select Wells or Load Coordinates), use Plate View if enabled
is_well_based = xy_mode is not None and xy_mode in ("Select Wells", "Load Coordinates")
if is_well_based and self.napariPlateViewWidget is not None and Nz == 1:
self.imageDisplayTabs.setCurrentWidget(self.napariPlateViewWidget)
elif USE_NAPARI_FOR_MOSAIC_DISPLAY and Nz == 1:
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing null check for napariMosaicDisplayWidget before use. When USE_NAPARI_FOR_MOSAIC_DISPLAY is False, napariMosaicDisplayWidget is set to None (line 795), but this code attempts to use it without checking. This will cause an AttributeError when trying to call setCurrentWidget with None. Add a null check similar to the napariPlateViewWidget check on line 1418.

Suggested change
elif USE_NAPARI_FOR_MOSAIC_DISPLAY and Nz == 1:
elif USE_NAPARI_FOR_MOSAIC_DISPLAY and self.napariMosaicDisplayWidget is not None and Nz == 1:

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@hongquanli hongquanli Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code] Skipped - false positive. If USE_NAPARI_FOR_MOSAIC_DISPLAY is False, the widget is None but the condition is also False, so setCurrentWidget is never called with None.

self.imageDisplayTabs.setCurrentWidget(self.napariMosaicDisplayWidget)
elif USE_NAPARI_FOR_MULTIPOINT:
self.imageDisplayTabs.setCurrentWidget(self.napariMultiChannelWidget)
else:
Expand Down
Loading