From 8ea811ebb23a8ff540d5468e47f6ce63426fa93a Mon Sep 17 00:00:00 2001 From: wyzula-jan Date: Tue, 4 Aug 2026 15:29:34 +0200 Subject: [PATCH] fix(scan_control): report the visible scans in the allowed_scans property --- .../control/scan_control/scan_control.py | 41 +++++-- tests/unit_tests/test_scan_control.py | 116 ++++++++++++++++-- 2 files changed, 140 insertions(+), 17 deletions(-) diff --git a/bec_widgets/widgets/control/scan_control/scan_control.py b/bec_widgets/widgets/control/scan_control/scan_control.py index 2c03c712..db92e19c 100644 --- a/bec_widgets/widgets/control/scan_control/scan_control.py +++ b/bec_widgets/widgets/control/scan_control/scan_control.py @@ -96,7 +96,9 @@ class ScanControl(BECWidget, QWidget): if default_scan is not None: self.config.default_scan = default_scan if allowed_scans is not None: - self.config.allowed_scans = allowed_scans + # Same normalization as the property setter: an empty filter means "no filter", + # so no construction argument can produce a selector without a runnable scan. + self.config.allowed_scans = list(dict.fromkeys(allowed_scans)) or None self._scan_metadata: dict | None = None self._metadata_form = ScanMetadata(parent=self) @@ -234,7 +236,7 @@ class ScanControl(BECWidget, QWidget): """Return available scans that can be rendered by this widget.""" return [ scan_name - for scan_name, scan_info in self.available_scans.items() + for scan_name, scan_info in getattr(self, "available_scans", {}).items() if scan_info.get("base_class") in self.SUPPORTED_SCAN_BASE_CLASSES and self._scan_info_adapter.has_scan_ui_config(scan_info) and not scan_name.startswith("_") @@ -245,7 +247,8 @@ class ScanControl(BECWidget, QWidget): current_scan = self.comboBox_scan_selection.currentText() if current_scan: self.save_current_scan_parameters() - allowed_scans = self.allowed_scans + # Read the raw filter: ``None`` means "unset", which the property never reports. + allowed_scans = self.config.allowed_scans if allowed_scans is None: visible_scans = self._supported_scan_names() else: @@ -287,7 +290,7 @@ class ScanControl(BECWidget, QWidget): def show_scan_selector_settings(self, *_): """Open the scan filter dialog and apply accepted changes.""" scan_names = self._supported_scan_names() - allowed_scans = self.allowed_scans + allowed_scans = self.config.allowed_scans if allowed_scans is not None: # Keep configured entries visible in the dialog even when currently unsupported. scan_names += [scan for scan in allowed_scans if scan not in scan_names] @@ -307,19 +310,39 @@ class ScanControl(BECWidget, QWidget): # Everything checked means "no filter", so scans added later show up as well. self.allowed_scans = None if selected_scans == scan_names else selected_scans - @SafeProperty(list) - def allowed_scans(self) -> list[str] | None: - """Scan filter for the selector; None shows every supported scan, including future ones.""" + @SafeProperty("QStringList") + def allowed_scans(self) -> list[str]: + """Scans configured for the selector. + + Reports the configured filter when one is set - entries that are not currently + available are kept, so a filter survives a scan disappearing and reappearing - + and every supported scan when no filter is set. It never reports ``None``: Qt + cannot convert that to a list property, so an unset filter would surface as + ``[]`` and, once stored in a profile and restored, would filter every scan away. + """ allowed_scans = getattr(self.config, "allowed_scans", None) - return None if allowed_scans is None else list(allowed_scans) + return self._supported_scan_names() if allowed_scans is None else list(allowed_scans) @allowed_scans.setter def allowed_scans(self, scan_names: list[str] | str | None): - """Set the scans displayed in the selector; None clears the filter.""" + """Set the scans displayed in the selector. + + ``None``, an empty list, or a list holding exactly the currently supported scans + in their listed order all clear the filter, so no configuration can leave the + selector without a single runnable scan. A reordered full list is kept as an + explicit filter, preserving the caller's ordering in the selector. + + Note that "no filter" and "every scan selected" are the same plain list once + stored: a profile saved before further scans became available is restored as an + explicit filter and does not pick those scans up until the selection is cleared + again (for example by checking everything in the scan filter dialog). + """ if isinstance(scan_names, str): scan_names = [scan_names] if scan_names is not None: scan_names = list(dict.fromkeys(scan_names)) + if not scan_names or scan_names == self._supported_scan_names(): + scan_names = None self.config.allowed_scans = scan_names self._update_scan_selector() diff --git a/tests/unit_tests/test_scan_control.py b/tests/unit_tests/test_scan_control.py index 141e4a92..c3c64a6f 100644 --- a/tests/unit_tests/test_scan_control.py +++ b/tests/unit_tests/test_scan_control.py @@ -361,8 +361,9 @@ def test_allowed_scans_none_clears_filter(scan_control): scan_control.allowed_scans = None - assert scan_control.allowed_scans is None + # the filter is unset internally, while the property reports what the selector shows assert scan_control.config.allowed_scans is None + assert scan_control.allowed_scans == ["line_scan", "grid_scan"] assert scan_control.comboBox_scan_selection.count() == 2 @@ -386,8 +387,9 @@ def test_filter_change_saves_current_scan_parameters(scan_control): assert "line_scan" in scan_control.config.scans -def test_empty_allowed_scans_disable_scan_info_and_run(scan_control): - scan_control.allowed_scans = [] +def test_allowed_scans_matching_nothing_disable_scan_info_and_run(scan_control): + # a filter whose entries are all unavailable leaves the selector empty + scan_control.allowed_scans = ["only_scan_that_is_not_available"] assert scan_control.comboBox_scan_selection.count() == 0 assert scan_control.comboBox_scan_selection.toolTip() == "" @@ -457,7 +459,7 @@ def test_scan_selector_settings_dialog_all_checked_clears_filter(scan_control, m qtbot.mouseClick(scan_control.scan_selector_settings_button, Qt.MouseButton.LeftButton) - assert scan_control.allowed_scans is None + assert scan_control.config.allowed_scans is None assert scan_control.comboBox_scan_selection.count() == 2 @@ -494,7 +496,7 @@ def test_scan_selector_dialog_select_all_clears_the_filter(scan_control, monkeyp qtbot.mouseClick(scan_control.scan_selector_settings_button, Qt.MouseButton.LeftButton) - assert scan_control.allowed_scans is None + assert scan_control.config.allowed_scans is None assert scan_control.comboBox_scan_selection.count() == 2 @@ -549,10 +551,14 @@ def test_scan_selector_dialog_info_button_opens_docs_without_toggling(qtbot): def test_scan_selector_settings_properties_are_profile_safe(scan_control): exported = scan_control.export_settings() - # "No filter" survives the round trip as None so that future scans keep appearing. - assert exported["allowed_scans"] is None + # "No filter" is exported as the full scan list, which the setter maps back onto an + # unset filter, so future scans keep appearing after a profile round trip. + assert exported["allowed_scans"] == ["line_scan", "grid_scan"] assert exported["hide_scan_selector_settings_button"] is False + scan_control.load_settings(exported) + assert scan_control.config.allowed_scans is None + scan_control.load_settings( {"allowed_scans": ["grid_scan"], "hide_scan_selector_settings_button": True} ) @@ -564,7 +570,7 @@ def test_scan_selector_settings_properties_are_profile_safe(scan_control): scan_control.load_settings({"allowed_scans": None}) - assert scan_control.allowed_scans is None + assert scan_control.config.allowed_scans is None assert scan_control.comboBox_scan_selection.count() == 2 @@ -1320,3 +1326,97 @@ def test_restore_parameters_with_fewer_arg_bundles(scan_control): args, kwargs = scan_control.get_scan_parameters(bec_object=False) assert args == ["samx", 0.0, 2.0] assert kwargs["steps"] == 10 + + +def test_allowed_scans_property_reports_selector_contents_on_init(scan_control): + """A freshly added widget must report the scans it shows, not an empty list. + + ``allowed_scans`` is a list-typed Qt property, and Qt cannot convert ``None`` to a + list: reporting the unset filter as ``None`` surfaced as ``[]`` through the property + system, which a profile then stored and restored as "allow nothing". + """ + visible_scans = [ + scan_control.comboBox_scan_selection.itemText(index) + for index in range(scan_control.comboBox_scan_selection.count()) + ] + + assert scan_control.config.allowed_scans is None + assert scan_control.allowed_scans == visible_scans + assert scan_control.property("allowed_scans") == visible_scans + + +def test_allowed_scans_property_round_trip_keeps_filter_unset(scan_control): + """Restoring the property in the same session must not turn it into a filter.""" + scan_control.load_settings({"allowed_scans": scan_control.property("allowed_scans")}) + + assert scan_control.config.allowed_scans is None + assert scan_control.comboBox_scan_selection.count() == 2 + + # a scan published later still shows up, i.e. the filter really is unset + scan_control.available_scans["extra_scan"] = scan_control.available_scans["line_scan"] + scan_control._update_scan_selector() + + assert scan_control.comboBox_scan_selection.count() == 3 + + +def test_allowed_scans_empty_list_restores_unfiltered_selector(scan_control): + """Profiles written before the fix hold an empty list; it must not blank the selector.""" + scan_control.allowed_scans = [] + + assert scan_control.config.allowed_scans is None + assert scan_control.comboBox_scan_selection.count() == 2 + assert scan_control.comboBox_scan_selection.currentText() == "line_scan" + assert scan_control.button_run_scan.isEnabled() + + +def test_allowed_scans_restored_after_new_scan_keeps_stored_selection(scan_control): + """A profile stored before a new scan appeared is restored as an explicit filter. + + "No filter" and "every scan selected" serialize to the same plain list, so the stored + selection wins and the newer scan stays hidden until the filter is cleared again. This + pins the documented limitation of the property rather than an intended feature. + """ + saved = scan_control.property("allowed_scans") + scan_control.available_scans["extra_scan"] = scan_control.available_scans["line_scan"] + + scan_control.load_settings({"allowed_scans": saved}) + + assert scan_control.config.allowed_scans == saved + assert scan_control.comboBox_scan_selection.count() == 2 + + # clearing the filter picks the newer scan up again + scan_control.allowed_scans = None + + assert scan_control.comboBox_scan_selection.count() == 3 + + +def test_allowed_scans_empty_at_construction_shows_all_scans(qtbot, mocked_client): + mocked_client.connector.set_and_publish( + MessageEndpoints.available_scans(), available_scans_message + ) + widget = ScanControl(client=mocked_client, allowed_scans=[]) + qtbot.addWidget(widget) + qtbot.waitExposed(widget) + + assert widget.config.allowed_scans is None + assert widget.comboBox_scan_selection.count() == 2 + assert widget.button_run_scan.isEnabled() or widget._scan_metadata is None + + +def test_allowed_scans_reordered_full_list_keeps_order(scan_control): + supported = scan_control._supported_scan_names() + reordered = list(reversed(supported)) + + scan_control.allowed_scans = reordered + + # a reordered full list is an explicit filter that preserves the caller's order + assert scan_control.config.allowed_scans == reordered + items = [ + scan_control.comboBox_scan_selection.itemText(i) + for i in range(scan_control.comboBox_scan_selection.count()) + ] + assert items == reordered + + # the same list in the supported order clears the filter + scan_control.allowed_scans = supported + assert scan_control.config.allowed_scans is None