Skip to content

Commit 733cd8f

Browse files
Add configurable GUI settings popup (#1244)
* Add GUI settings popup * Align GUI setting action buttons * update tests * Apply display settings without restart * fix issue with missing display config * Synchronize spinbox precision with increment * Use configured stack step minimum * Fix GUI settings validation * Harden GUI settings persistence and stack validation Prevent custom GUI settings from exposing unsafe or surprising behavior during normal operation. Changes: - Reject non-positive z-stack step sizes before division, independently of the configured widget minimum. - Save GUI settings back to the exact file supplied through the GUI configuration path while leaving every other configuration save target unchanged. - Add headless regression coverage for a zero step with a zero configured minimum. - Add exit-path coverage proving custom GUI settings are written to their source file. - Normalize the touched channel controller with the repository-pinned formatter. Verification: - 44 focused controller and GUI settings tests passed. - Black 22.10.0 passed on all modified files. - Ruff 0.0.191 passed on all modified files. - git diff --check passed. --------- Co-authored-by: Kevin M. Dean <kevin.dean@utsouthwestern.edu>
1 parent dfa4d53 commit 733cd8f

23 files changed

Lines changed: 1086 additions & 67 deletions

‎src/navigate/config/config.py‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,17 @@
5252
p = __name__.split(".")[1]
5353
logger = logging.getLogger(p)
5454

55+
GUI_SETTING_DEFAULTS = {
56+
"remote_focus_waveform": {
57+
"amplitude_step_size": 0.0001,
58+
"offset_step_size": 0.0001,
59+
},
60+
"galvo_waveform": {
61+
"amplitude_step_size": 0.0001,
62+
"offset_step_size": 0.0001,
63+
},
64+
}
65+
5566

5667
def get_navigate_path():
5768
"""Establish a program home directory in AppData/Local/.navigate for Windows
@@ -619,7 +630,7 @@ def verify_experiment_config(manager, configuration):
619630
].get("filter_wheel", [])
620631
]
621632
prefix = "channel_"
622-
channel_nums = configuration["configuration"]["gui"]["channels"]["count"]
633+
channel_nums = configuration["gui"].get("channel_settings", {}).get("count", 5)
623634
channel_setting_dict = microscope_setting_dict["channels"]
624635
selected_channel_num = 0
625636
for channel in channel_setting_dict.keys():
@@ -1093,12 +1104,17 @@ def verify_configuration(manager, configuration):
10931104
idx = ref_list["filter_wheel"].index(filter_wheel_idx)
10941105
temp_config[i]["name"] = filter_wheel_seq[idx]["name"]
10951106

1096-
update_config_dict(
1097-
manager,
1098-
configuration["configuration"],
1099-
"gui",
1100-
{"channels": {"count": channel_count}},
1101-
)
1107+
gui_settings = configuration["gui"]
1108+
if "channel_settings" not in gui_settings:
1109+
update_config_dict(manager, gui_settings, "channel_settings", {})
1110+
gui_settings["channel_settings"]["count"] = channel_count
1111+
for group_name, defaults in GUI_SETTING_DEFAULTS.items():
1112+
if group_name not in gui_settings:
1113+
update_config_dict(manager, gui_settings, group_name, defaults)
1114+
continue
1115+
for setting_name, value in defaults.items():
1116+
if setting_name not in gui_settings[group_name]:
1117+
gui_settings[group_name][setting_name] = value
11021118

11031119

11041120
def verify_positions_config(positions):

‎src/navigate/config/gui_configuration.yml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,9 @@ time:
7777
step: 1
7878
min: 1
7979
max: 5000
80+
remote_focus_waveform:
81+
amplitude_step_size: 0.0001
82+
offset_step_size: 0.0001
83+
galvo_waveform:
84+
amplitude_step_size: 0.0001
85+
offset_step_size: 0.0001

‎src/navigate/controller/configuration_controller.py‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -526,12 +526,7 @@ def number_of_channels(self) -> int:
526526
number_of_channels : int
527527
Number of channels.
528528
"""
529-
if self.microscope_config is not None:
530-
number_of_channels = (
531-
self.configuration["gui"].get("channel_settings", {}).get("count", 5)
532-
)
533-
return number_of_channels
534-
return 5
529+
return self.configuration["gui"].get("channel_settings", {}).get("count", 5)
535530

536531
@property
537532
def number_of_filter_wheels(self) -> int:
@@ -658,7 +653,7 @@ def gui_setting(self) -> dict:
658653
gui_setting : dict
659654
Dictionary with the GUI settings.
660655
"""
661-
return self.configuration["configuration"]["gui"]
656+
return self.configuration["gui"]
662657

663658
def is_same_camera(self, microscope_name: str) -> bool:
664659
"""Check if the current microscope uses the same camera as the given microscope.

‎src/navigate/controller/controller.py‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1462,8 +1462,13 @@ def execute(self, command: str, *args: Any) -> Any:
14621462
("rest_api_config", "rest_api_config.yml"),
14631463
("waveform_templates", "waveform_templates.yml"),
14641464
]:
1465+
config_directory = file_directory
1466+
if config_name == "gui":
1467+
gui_configuration_path = os.fspath(self.gui_configuration_path)
1468+
config_directory = os.path.dirname(gui_configuration_path)
1469+
filename = os.path.basename(gui_configuration_path)
14651470
save_yaml_file(
1466-
file_directory=file_directory,
1471+
file_directory=config_directory,
14671472
content_dict=self.configuration[config_name],
14681473
filename=filename,
14691474
)

‎src/navigate/controller/sub_controllers/channels_settings.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ def update_setting_dict(setting_dict, widget_name):
375375
# else widget_name
376376
# )
377377
# setting_range = self.parent_controller.parent_controller.
378-
# configuration["configuration"]["gui"]["channels"][ref_name]
378+
# configuration["gui"]["channel_settings"][ref_name]
379379
# if (
380380
# setting_dict[widget_name] < setting_range["min"]
381381
# or setting_dict[widget_name] > setting_range["max"]

‎src/navigate/controller/sub_controllers/channels_tab.py‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -319,9 +319,9 @@ def populate_experiment_values(self) -> None:
319319
def set_spinbox_range_limits(self, settings: Dict[str, Any]) -> None:
320320
"""Sets range limits for various spinbox widgets based on the provided settings.
321321
322-
This method configures the minimum, maximum, and increment values for the spinbox
323-
widgets in the stack acquisition settings frame and the time point settings
324-
frame based on the provided configuration settings.
322+
This method configures the minimum, maximum, and increment values for the
323+
spinbox widgets in the stack acquisition settings frame and the time point
324+
settings frame based on the provided configuration settings.
325325
326326
Parameters
327327
----------
@@ -568,7 +568,10 @@ def update_z_steps(self, *_: tuple[str]) -> None:
568568
start_position = float(self.stack_acq_vals["start_position"].get())
569569
end_position = float(self.stack_acq_vals["end_position"].get())
570570
step_size = float(self.stack_acq_vals["step_size"].get())
571-
if step_size < 0.001:
571+
# Reject non-positive values independently of the configured minimum.
572+
if step_size <= 0 or step_size < self.stack_acq_widgets[
573+
"step_size"
574+
].widget.cget("from"):
572575
self.stack_acq_vals["number_z_steps"].set(0)
573576
self.microscope_state_dict["abs_z_start"] = 0
574577
self.microscope_state_dict["abs_z_end"] = 0
@@ -1018,9 +1021,9 @@ def update_experiment_values(self) -> None:
10181021
offset = variable_dict[f"{axis}_offset"].get()
10191022
secondary_stack_settings[axis] = offset
10201023

1021-
self.microscope_state_dict["secondary_stack_settings"] = (
1022-
secondary_stack_settings
1023-
)
1024+
self.microscope_state_dict[
1025+
"secondary_stack_settings"
1026+
] = secondary_stack_settings
10241027

10251028
def verify_experiment_values(self) -> str:
10261029
"""Verify channel tab settings and return warning info
@@ -1131,9 +1134,9 @@ def set_defocus_reference(self, reference) -> None:
11311134
f"Defocus Reference: {channel_label} @ {focus_position:.2f}"
11321135
)
11331136
if reference_channel is None:
1134-
self.parent_controller.configuration["experiment"][
1135-
"AutoFocusParameters"
1136-
]["reference_channel"] = channel_label
1137+
self.parent_controller.configuration["experiment"]["AutoFocusParameters"][
1138+
"reference_channel"
1139+
] = channel_label
11371140

11381141
def update_additional_stacking_axes(self, *args, **kwargs):
11391142
if self.stack_acq_vals["z_device"].get():
Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,235 @@
1+
# Copyright (c) 2021-2026 The University of Texas Southwestern Medical Center.
2+
# All rights reserved.
3+
4+
"""Controller for the GUI settings popup."""
5+
6+
from collections.abc import Mapping, Sequence
7+
from math import isfinite
8+
from typing import Any
9+
10+
from navigate.view.popups.gui_settings_popup import GuiSettingsPopup
11+
12+
13+
def iter_gui_settings(
14+
settings: Any, path: tuple[str, ...] = ()
15+
) -> list[tuple[tuple[str, ...], Any]]:
16+
"""Return non-theme GUI configuration leaves and their paths."""
17+
if isinstance(settings, Mapping) or hasattr(settings, "items"):
18+
rows: list[tuple[tuple[str, ...], Any]] = []
19+
for key, value in settings.items():
20+
key = str(key)
21+
if not path and key == "theme":
22+
continue
23+
rows.extend(iter_gui_settings(value, (*path, key)))
24+
return rows
25+
if isinstance(settings, Sequence) and not isinstance(settings, (str, bytes)):
26+
return [(path, list(settings))]
27+
return [(path, settings)]
28+
29+
30+
def flatten_gui_settings(settings: Any) -> list[tuple[str, str]]:
31+
"""Return non-theme GUI settings as dotted path/value pairs."""
32+
return [(".".join(path), str(value)) for path, value in iter_gui_settings(settings)]
33+
34+
35+
def is_integer_gui_setting(path: tuple[str, ...]) -> bool:
36+
"""Return whether a GUI setting must use an integer entry."""
37+
return path == ("channel_settings", "count") or path[:2] == (
38+
"time",
39+
"timepoints",
40+
)
41+
42+
43+
def is_boolean_gui_setting(path: tuple[str, ...]) -> bool:
44+
"""Return whether a GUI setting is represented by a checkbutton."""
45+
return path in {("histogram", "enabled"), ("mip_display", "enabled")}
46+
47+
48+
def is_waveform_gui_setting(path: tuple[str, ...]) -> bool:
49+
"""Return whether a setting affects waveform popup control increments."""
50+
return path[0] in {"remote_focus_waveform", "galvo_waveform"}
51+
52+
53+
def is_step_size_gui_setting(path: tuple[str, ...]) -> bool:
54+
"""Return whether a GUI setting defines a numeric step size."""
55+
return path[-1] == "step" or path[-1].endswith("_step_size")
56+
57+
58+
def is_positive_gui_setting(path: tuple[str, ...]) -> bool:
59+
"""Return whether a GUI setting must be strictly greater than zero."""
60+
return is_step_size_gui_setting(path) or path[:2] == (
61+
"stack_acquisition",
62+
"step_size",
63+
)
64+
65+
66+
def is_nonnegative_gui_setting(path: tuple[str, ...]) -> bool:
67+
"""Return whether a GUI setting is constrained to non-negative values."""
68+
return (
69+
path[:2]
70+
in {
71+
("channel_settings", "laser_power"),
72+
("channel_settings", "exposure_time"),
73+
("channel_settings", "interval"),
74+
}
75+
)
76+
77+
78+
def gui_setting_minimum(path: tuple[str, ...]) -> str:
79+
"""Return the entry minimum for GUI settings with lower-bound constraints."""
80+
if path == ("channel_settings", "count"):
81+
return "1"
82+
if is_positive_gui_setting(path):
83+
return "0.000000001"
84+
if is_nonnegative_gui_setting(path) or path[0] == "time":
85+
return "0"
86+
return "-Infinity"
87+
88+
89+
def gui_setting_group(path: tuple[str, ...]) -> str:
90+
"""Return the human-readable group title for a GUI setting path."""
91+
group_names = {
92+
"remote_focus_waveform": "Remote Focus Waveform",
93+
"galvo_waveform": "Galvo Waveform",
94+
}
95+
return group_names.get(path[0], path[0].replace("_", " ").title())
96+
97+
98+
def coerce_gui_value(value: str, path: tuple[str, ...]) -> Any:
99+
"""Convert text entered in the popup to the value type for a GUI setting."""
100+
parsed_value = int(value) if is_integer_gui_setting(path) else float(value)
101+
if not is_integer_gui_setting(path) and not isfinite(parsed_value):
102+
raise ValueError("Value must be finite")
103+
if path == ("channel_settings", "count") and parsed_value <= 0:
104+
raise ValueError("Value must be greater than 0")
105+
if is_positive_gui_setting(path) and parsed_value <= 0:
106+
raise ValueError("Value must be greater than 0")
107+
if (path[0] == "time" or is_nonnegative_gui_setting(path)) and parsed_value < 0:
108+
raise ValueError("Value must be greater than or equal to 0")
109+
return parsed_value
110+
111+
112+
class GuiSettingsPopupController:
113+
"""Connect the GUI settings view to the shared GUI configuration."""
114+
115+
def __init__(self, view: GuiSettingsPopup, parent_controller: "Controller") -> None:
116+
self.view = view
117+
self.parent_controller = parent_controller
118+
self.view.buttons["apply"].configure(command=self.apply_settings)
119+
self.view.buttons["close"].configure(command=self.close_popup)
120+
self.view.popup.protocol("WM_DELETE_WINDOW", self.close_popup)
121+
self.populate_settings()
122+
123+
def populate_settings(self) -> None:
124+
"""Populate the view from the active GUI configuration."""
125+
fields = []
126+
for path, value in iter_gui_settings(self.parent_controller.configuration["gui"]):
127+
fields.append(
128+
{
129+
"path": path,
130+
"value": value,
131+
"group": gui_setting_group(path),
132+
"label": ".".join(path[1:]).replace("_", " "),
133+
"minimum": gui_setting_minimum(path),
134+
"type": (
135+
"boolean"
136+
if is_boolean_gui_setting(path)
137+
else "integer"
138+
if is_integer_gui_setting(path)
139+
else "float"
140+
),
141+
}
142+
)
143+
self.view.populate_settings(fields)
144+
145+
def apply_settings(self) -> bool:
146+
"""Validate and write user edits to the shared GUI configuration."""
147+
updates: list[tuple[tuple[str, ...], Any]] = []
148+
validation_errors: list[tuple[tuple[str, ...], Any, ValueError]] = []
149+
for path, (value_var, entry) in self.view.entries.items():
150+
try:
151+
updates.append((path, coerce_gui_value(value_var.get(), path)))
152+
entry._toggle_error(False)
153+
except ValueError as error:
154+
entry._toggle_error(True)
155+
validation_errors.append((path, entry, error))
156+
if validation_errors:
157+
path, entry, error = validation_errors[0]
158+
self.view.set_status(f"{'.'.join(path)}: {error}")
159+
entry.focus_set()
160+
return False
161+
updates.extend(
162+
(path, value_var.get())
163+
for path, value_var in self.view.boolean_variables.items()
164+
)
165+
166+
waveform_settings_changed = False
167+
other_settings_changed = False
168+
display_setting_updates: list[tuple[tuple[str, ...], bool]] = []
169+
for path, value in updates:
170+
parent = self.parent_controller.configuration["gui"]
171+
for key in path[:-1]:
172+
parent = parent[key]
173+
if parent[path[-1]] != value:
174+
if is_waveform_gui_setting(path):
175+
waveform_settings_changed = True
176+
elif is_boolean_gui_setting(path):
177+
display_setting_updates.append((path, value))
178+
else:
179+
other_settings_changed = True
180+
parent[path[-1]] = value
181+
182+
self._apply_display_setting_updates(display_setting_updates)
183+
messages = []
184+
if waveform_settings_changed:
185+
waveform_popup = getattr(
186+
self.parent_controller, "waveform_popup_controller", None
187+
)
188+
messages.append("Waveform step-size settings were applied successfully.")
189+
if waveform_popup is not None:
190+
messages.append(
191+
"The Waveform Parameters window was reopened to take effect"
192+
)
193+
self._refresh_waveform_popup(waveform_popup)
194+
if other_settings_changed:
195+
messages.append(
196+
"Setting changes were saved. Restart Navigate for the changes "
197+
"to take effect."
198+
)
199+
if messages:
200+
self.view.show_info("Settings Saved", "\n\n".join(messages))
201+
return True
202+
203+
def _apply_display_setting_updates(
204+
self, updates: list[tuple[tuple[str, ...], bool]]
205+
) -> None:
206+
"""Apply display toggles through the existing menu-controller workflow."""
207+
menu_controller = self.parent_controller.menu_controller
208+
for path, value in updates:
209+
if path == ("histogram", "enabled"):
210+
menu_controller.histogram_enabled.set(value)
211+
menu_controller.toggle_histogram()
212+
elif path == ("mip_display", "enabled"):
213+
menu_controller.mip_enabled.set(value)
214+
menu_controller.toggle_mip()
215+
216+
def _refresh_waveform_popup(self, waveform_popup=None) -> None:
217+
"""Reopen an existing waveform popup so it uses the updated increments."""
218+
if waveform_popup is None:
219+
waveform_popup = getattr(
220+
self.parent_controller, "waveform_popup_controller", None
221+
)
222+
if waveform_popup is None:
223+
return
224+
waveform_popup.close_window()
225+
self.parent_controller.menu_controller.popup_waveform_setting()
226+
227+
def showup(self) -> None:
228+
"""Show the existing settings window."""
229+
self.view.showup()
230+
231+
def close_popup(self) -> None:
232+
"""Close the popup and discard its controller reference."""
233+
self.view.popup.destroy()
234+
if hasattr(self.parent_controller, "gui_settings_popup_controller"):
235+
del self.parent_controller.gui_settings_popup_controller

0 commit comments

Comments
 (0)