From d64d32146cef73dff036eee410d9ed9c6853e908 Mon Sep 17 00:00:00 2001 From: Marcos Frenkel Date: Tue, 20 Oct 2026 14:59:43 -0500 Subject: [PATCH 01/10] Parameter Manager GUI: add Remove type button to the Types pane The button shows while a Type is selected and calls remove_type on the server; a refused removal shows the error on the New type strip's note. Co-Authored-By: Claude Opus 5.5 --- .../gui/parameter_manager/panels.py | 19 +++++++++++++++++++ .../gui/parameter_manager/widget.py | 15 +++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/src/instrumentserver/gui/parameter_manager/panels.py b/src/instrumentserver/gui/parameter_manager/panels.py index c52967d..b0aabd2 100644 --- a/src/instrumentserver/gui/parameter_manager/panels.py +++ b/src/instrumentserver/gui/parameter_manager/panels.py @@ -638,6 +638,11 @@ class TypesPane(QtWidgets.QWidget): #: the name is trimmed and not empty. addTypeRequested = QtCore.Signal(str) + #: Signal(str) + #: Emitted when the user presses the Remove type button: the selected + #: Type's name. + removeTypeRequested = QtCore.Signal(str) + #: Signal(str) #: Emitted when the selected Type changes (a row click, or a rebuild #: that had to pick one). @@ -746,6 +751,16 @@ def __init__( keepSmallHorizontally(self.addTypeButton) typeStrip.addWidget(self.newTypeEdit, 1) typeStrip.addWidget(self.addTypeButton) + self.removeTypeButton = QtWidgets.QPushButton( + QtGui.QIcon(":/icons/delete.svg"), " Remove" + ) + self.removeTypeButton.setStyleSheet("QPushButton { background-color: salmon }") + self.removeTypeButton.setToolTip( + "remove the selected Type — the Instances keep their parameters" + ) + self.removeTypeButton.setVisible(False) + keepSmallHorizontally(self.removeTypeButton) + typeStrip.addWidget(self.removeTypeButton) self.typeNote = QtWidgets.QLabel(typeListPane) typeListLayout.addWidget(self.typeList, 1) @@ -888,6 +903,9 @@ def __init__( self.addTypeButton.clicked.connect(self._request_add_type) self.newTypeEdit.returnPressed.connect(self.addTypeButton.click) + self.removeTypeButton.clicked.connect( + lambda: self.removeTypeRequested.emit(self.selectedType) + ) self.addEntryButton.clicked.connect(self._request_add_entry) for edit in (self.entryNameEdit, self.entryDefaultEdit, self.entryUnitEdit): edit.returnPressed.connect(self.addEntryButton.click) @@ -979,6 +997,7 @@ def _rebuild_selected_panes( self.instancesLabel.setText( f"instances of {selected}" if has_type else "instances" ) + self.removeTypeButton.setVisible(has_type) self.addEntryButton.setEnabled(has_type) self.addNestedButton.setEnabled(has_type) self.addInstanceButton.setEnabled(has_type) diff --git a/src/instrumentserver/gui/parameter_manager/widget.py b/src/instrumentserver/gui/parameter_manager/widget.py index f2f4b5c..9fe2f83 100644 --- a/src/instrumentserver/gui/parameter_manager/widget.py +++ b/src/instrumentserver/gui/parameter_manager/widget.py @@ -980,6 +980,7 @@ def connectSignals(self) -> None: # the two panes it drives gui.typesPane.typeSelected.connect(self._on_pane_type_selected) gui.typesPane.addTypeRequested.connect(self._on_pane_add_type) + gui.typesPane.removeTypeRequested.connect(self._on_pane_remove_type) gui.typesPane.addEntryRequested.connect(self._on_pane_add_entry) gui.typesPane.removeEntryRequested.connect(self._on_pane_remove_entry) gui.typesPane.setDefaultRequested.connect(self._on_pane_set_default) @@ -1094,6 +1095,20 @@ def _on_pane_add_type(self, name: str) -> None: self.gui.typesPane.select_type(name) self.refresh_types_pane() + @QtCore.Slot(str) + def _on_pane_remove_type(self, name: str) -> None: + """The Remove type button: remove the selected Type (D13) — the + Instances keep their parameters. A refused removal (the Type is + nested in another) shows the Server's error text on the strip's + note.""" + try: + self.gui.instrument.remove_type(name) + except Exception as exc: + self.gui.typesPane.show_type_error(str(exc)) + else: + self.gui.typesPane.reset_type_note() + self.refresh_types_pane() + @QtCore.Slot(str, str, str, str) def _on_pane_add_entry( self, type_name: str, path: str, default_text: str, unit: str From d44217b73c352c4b93f6ebb8f9dcf69e88d7ebe6 Mon Sep 17 00:00:00 2001 From: Marcos Frenkel Date: Tue, 20 Oct 2026 15:12:58 -0500 Subject: [PATCH 02/10] Parameter Manager GUI: make a Type entry's unit editable in the Types pane An own entry's unit cell is a line edit; Return calls set_type_parameter_unit, which propagates the unit to every Instance, then refreshes the GUI so the parameter tree shows the new units. Co-Authored-By: Claude Opus 5.5 --- .../gui/parameter_manager/panels.py | 20 +++++++++++++++++++ .../gui/parameter_manager/widget.py | 16 +++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/src/instrumentserver/gui/parameter_manager/panels.py b/src/instrumentserver/gui/parameter_manager/panels.py index b0aabd2..05ee812 100644 --- a/src/instrumentserver/gui/parameter_manager/panels.py +++ b/src/instrumentserver/gui/parameter_manager/panels.py @@ -664,6 +664,11 @@ class TypesPane(QtWidgets.QWidget): #: the editor's text. setDefaultRequested = QtCore.Signal(str, str, str) + #: Signal(str, str, str) + #: Emitted when the user commits an own entry's unit editor (Return): + #: the Type's name, the entry path and the editor's text. + setUnitRequested = QtCore.Signal(str, str, str) + #: Signal(str, str) #: Emitted when the user presses a Type Lock toggle: the Type's name #: and the entry path. The GUI locks or unlocks from the entry's @@ -1098,6 +1103,21 @@ def _rebuild_entries( ) else: self._build_entry_row(selected, row, lock_item, default_item, entry) + if row.own: + # the editable unit: Return commits it + unit_editor = QtWidgets.QLineEdit( + row.unit, self.entriesView.viewport() + ) + unit_editor.returnPressed.connect( + lambda path=path, unit_editor=unit_editor: ( + self.setUnitRequested.emit( + selected, path, unit_editor.text() + ) + ) + ) + self.entriesView.setIndexWidget( + self.entriesModel.indexFromItem(unit_item), unit_editor + ) self.entriesView.expandAll() def _build_submodule_row( diff --git a/src/instrumentserver/gui/parameter_manager/widget.py b/src/instrumentserver/gui/parameter_manager/widget.py index 9fe2f83..18ed1e4 100644 --- a/src/instrumentserver/gui/parameter_manager/widget.py +++ b/src/instrumentserver/gui/parameter_manager/widget.py @@ -984,6 +984,7 @@ def connectSignals(self) -> None: gui.typesPane.addEntryRequested.connect(self._on_pane_add_entry) gui.typesPane.removeEntryRequested.connect(self._on_pane_remove_entry) gui.typesPane.setDefaultRequested.connect(self._on_pane_set_default) + gui.typesPane.setUnitRequested.connect(self._on_pane_set_unit) gui.typesPane.toggleTypeLockRequested.connect(self._on_pane_toggle_type_lock) gui.typesPane.addNestedRequested.connect(self._on_pane_add_nested) gui.typesPane.removeNestedRequested.connect(self._on_pane_remove_nested) @@ -1158,6 +1159,21 @@ def _on_pane_set_default(self, type_name: str, path: str, text: str) -> None: self.gui.typesPane.reset_entries_note() self.refresh_types_pane() + @QtCore.Slot(str, str, str) + def _on_pane_set_unit(self, type_name: str, path: str, unit: str) -> None: + """An own entry's committed unit editor (Return): set the entry's + unit, which the Server propagates to every Instance (D13). The + propagation broadcasts no parameter update, so the whole GUI is + refreshed to show the Instances' new units. A refused set shows + the Server's error text on the entries pane's note.""" + try: + self.gui.instrument.set_type_parameter_unit(type_name, path, unit.strip()) + except Exception as exc: + self.gui.typesPane.show_entries_error(str(exc)) + else: + self.gui.typesPane.reset_entries_note() + self.gui.refreshAll() + @QtCore.Slot(str, str) def _on_pane_toggle_type_lock(self, type_name: str, path: str) -> None: """An entry's Type Lock toggle: declare the Type Lock on the From 3e996457c6037e5df2e542b5dc3391c3bd6cca58 Mon Sep 17 00:00:00 2001 From: Marcos Frenkel Date: Tue, 20 Oct 2026 15:50:48 -0500 Subject: [PATCH 03/10] Parameter Manager GUI: add a delete button to each row of the instances pane The button sits next to Show; after confirmation it removes every parameter under the Instance, and the emptied groups go with them. Co-Authored-By: Claude Opus 5.5 --- .../gui/parameter_manager/panels.py | 25 +++++++++++++++-- .../gui/parameter_manager/widget.py | 28 +++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/instrumentserver/gui/parameter_manager/panels.py b/src/instrumentserver/gui/parameter_manager/panels.py index 05ee812..6d87576 100644 --- a/src/instrumentserver/gui/parameter_manager/panels.py +++ b/src/instrumentserver/gui/parameter_manager/panels.py @@ -602,7 +602,7 @@ def _build_row_widgets( #: button columns (the mock's 150/160/110 trio). INSTANCES_COUNT_WIDTH = 110 INSTANCES_ALSO_WIDTH = 160 -INSTANCES_BUTTON_WIDTH = 80 +INSTANCES_BUTTON_WIDTH = 110 class TypesPane(QtWidgets.QWidget): @@ -700,6 +700,11 @@ class TypesPane(QtWidgets.QWidget): #: Type's name and the Instance's name. showInstanceRequested = QtCore.Signal(str, str) + #: Signal(str) + #: Emitted when the user presses an instance row's delete button: the + #: Instance's name. + removeInstanceRequested = QtCore.Signal(str) + def __init__( self, instrument_name: str, parent: Optional[QtWidgets.QWidget] = None ) -> None: @@ -1318,15 +1323,29 @@ def _rebuild_instances( if colours is not None: for item in (name_item, count_item, also_item, button_item): item.setData(colours["tint"], QtCore.Qt.ItemDataRole.BackgroundRole) - show = QtWidgets.QPushButton("Show", parent=self.instancesView.viewport()) + buttons = QtWidgets.QWidget(self.instancesView.viewport()) + buttons_layout = QtWidgets.QHBoxLayout(buttons) + buttons_layout.setContentsMargins(0, 0, 0, 0) + show = QtWidgets.QPushButton("Show", parent=buttons) show.setToolTip("show in the parameter tree") show.pressed.connect( lambda type_name=selected, node=instance: ( self.showInstanceRequested.emit(type_name, node) ) ) + remove = QtWidgets.QPushButton( + QtGui.QIcon(":/icons/delete.svg"), "", parent=buttons + ) + remove.setStyleSheet("QPushButton { background-color: salmon }") + remove.setToolTip("delete the Instance and all its parameters") + keepSmallHorizontally(remove) + remove.pressed.connect( + lambda node=instance: self.removeInstanceRequested.emit(node) + ) + buttons_layout.addWidget(show, 1) + buttons_layout.addWidget(remove) self.instancesView.setIndexWidget( - self.instancesModel.indexFromItem(button_item), show + self.instancesModel.indexFromItem(button_item), buttons ) self.showButtons[instance] = show diff --git a/src/instrumentserver/gui/parameter_manager/widget.py b/src/instrumentserver/gui/parameter_manager/widget.py index 18ed1e4..d954591 100644 --- a/src/instrumentserver/gui/parameter_manager/widget.py +++ b/src/instrumentserver/gui/parameter_manager/widget.py @@ -990,6 +990,7 @@ def connectSignals(self) -> None: gui.typesPane.removeNestedRequested.connect(self._on_pane_remove_nested) gui.typesPane.addInstanceRequested.connect(self._on_pane_add_instance) gui.typesPane.showInstanceRequested.connect(self._on_pane_show_instance) + gui.typesPane.removeInstanceRequested.connect(self._on_pane_remove_instance) @QtCore.Slot(str, object) def _on_type_changed( @@ -1241,6 +1242,33 @@ def _on_pane_add_instance(self, type_name: str, name: str) -> None: self.gui.typesPane.newInstanceEdit.clear() self.refresh_types_pane() + @QtCore.Slot(str) + def _on_pane_remove_instance(self, instance: str) -> None: + """An instance row's delete button: after confirmation, remove + every parameter under the Instance; the emptied Parameter Groups + go with them. A refused removal shows the Server's error text on + the instances pane's note.""" + paths = [ + path + for path in self.gui.model.parameter_units() + if path.startswith(f"{instance}.") + ] + answer = QtWidgets.QMessageBox.question( + self.gui, + "Delete instance?", + f"Delete {instance} and its {len(paths)} parameters?", + ) + if answer != QtWidgets.QMessageBox.StandardButton.Yes: + return + try: + for path in paths: + self.gui.instrument.remove_parameter(path) + except Exception as exc: + self.gui.typesPane.show_instances_error(str(exc)) + else: + self.gui.typesPane.reset_instances_note() + self.refresh_types_pane() + @QtCore.Slot(str, str) def _on_pane_show_instance(self, type_name: str, instance: str) -> None: """An instance row's Show button: switch to the Parameters tab, From d04c7b21003fb279a168175ee49f35d1845eb1d7 Mon Sep 17 00:00:00 2001 From: Marcos Frenkel Date: Tue, 20 Oct 2026 16:46:49 -0500 Subject: [PATCH 04/10] Server and Parameter Manager GUI: fix idle CPU spin and slow instance edits - server: the zmq poller reports the wakeup socketpair by its file descriptor, so the old 'socket in socks' check never matched, the pipe was never drained, and the request loop spun at 100% CPU after the first request. - GUI: a burst of Broadcasts (an Instance added or removed) emits structureChanged once, after 50 ms of quiet, instead of rebuilding the tints and Types pane once per parameter. - GUI: the instance delete button calls the Server directly and refreshes the proxy once, instead of refetching the whole blueprint per parameter. Co-Authored-By: Claude Opus 5.5 --- .../gui/parameter_manager/widget.py | 19 +++++++++++++++++-- src/instrumentserver/server/core.py | 4 +++- test/pytest/test_pm_gui.py | 14 ++++++++++++-- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/src/instrumentserver/gui/parameter_manager/widget.py b/src/instrumentserver/gui/parameter_manager/widget.py index d954591..c5b2d5c 100644 --- a/src/instrumentserver/gui/parameter_manager/widget.py +++ b/src/instrumentserver/gui/parameter_manager/widget.py @@ -278,6 +278,12 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: # or reloads never reach updateParameter's check self._rows_added = False self.newItem.connect(self._on_new_item) + # a burst of Broadcasts (an Instance created or removed) emits + # structureChanged once, after the burst + self._structureTimer = QtCore.QTimer(self) + self._structureTimer.setSingleShot(True) + self._structureTimer.setInterval(50) + self._structureTimer.timeout.connect(self.structureChanged.emit) @QtCore.Slot(object) def _on_new_item(self, item: QtGui.QStandardItem) -> None: @@ -323,7 +329,7 @@ def updateParameter(self, bp: ParameterBroadcastBluePrint) -> None: # must be recomputed then (plan task 5.6), or the new row would # stay untinted until the next recompute if self._rows_added or bp.action == PARAMETER_DELETION: - self.structureChanged.emit() + self._structureTimer.start() class LockableParameterWidget(ParameterWidget): @@ -1260,9 +1266,18 @@ def _on_pane_remove_instance(self, instance: str) -> None: ) if answer != QtWidgets.QMessageBox.StandardButton.Yes: return + # call the Server directly and refresh the proxy once: the proxy's + # remove_parameter refetches the whole blueprint after every call + instrument = self.gui.instrument + cli = getattr(instrument, "cli", None) try: for path in paths: - self.gui.instrument.remove_parameter(path) + if cli is None: + instrument.remove_parameter(path) + else: + cli.call(f"{instrument.remotePath}.remove_parameter", path) + if cli is not None: + instrument.update() except Exception as exc: self.gui.typesPane.show_instances_error(str(exc)) else: diff --git a/src/instrumentserver/server/core.py b/src/instrumentserver/server/core.py index 2115730..c6cdaaf 100644 --- a/src/instrumentserver/server/core.py +++ b/src/instrumentserver/server/core.py @@ -262,7 +262,9 @@ def startServer(self) -> bool: pool.submit(self._handleRouterMessage, identity, message) # handle wakeup events (one or more workers finished) - if self._wakeup_r in socks and (socks[self._wakeup_r] & zmq.POLLIN): + # the poller reports a plain socket by its file descriptor, + # not by the socket object + if socks.get(self._wakeup_r.fileno(), 0) & zmq.POLLIN: # Drain the wakeup pipe so it doesn't stay "always readable" try: # Read whatever is there; content doesn't matter diff --git a/test/pytest/test_pm_gui.py b/test/pytest/test_pm_gui.py index 8d67fd7..d97b576 100644 --- a/test/pytest/test_pm_gui.py +++ b/test/pytest/test_pm_gui.py @@ -3588,8 +3588,8 @@ def test_a_parameter_update_for_an_unknown_row_recomputes_the_tints( """A parameter-update or parameter-call Broadcast for a parameter the model does not know adds the row through the base update branch; the tints are recomputed for it too (plan task 5.6), so the new row - carries the claiming Type's tint right away instead of staying - untinted until the next recompute.""" + carries the claiming Type's tint once the burst of Broadcasts is over + instead of staying untinted until the next recompute.""" second_pm = _second_parameter_manager(second_client) pm.add_parameter("q01.IF", initial_value=1.0, unit="Hz") pm.add_type("qubit") @@ -3619,6 +3619,11 @@ def test_a_parameter_update_for_an_unknown_row_recomputes_the_tints( ) ) qtbot.waitUntil(lambda: _row_exists(gui, "q02.IF"), timeout=BROADCAST_TIMEOUT) + # the recompute runs once the burst of Broadcasts is over + qtbot.waitUntil( + lambda: _row_items(gui, "q02.IF")[3].data(GUTTER_ROLE) == ["qubit"], + timeout=BROADCAST_TIMEOUT, + ) tint = _type_tint(gui, "qubit") assert tint is not None for item in _row_items(gui, "q02.IF"): @@ -3636,6 +3641,11 @@ def test_a_parameter_update_for_an_unknown_row_recomputes_the_tints( ) ) qtbot.waitUntil(lambda: _row_exists(gui, "q03.IF"), timeout=BROADCAST_TIMEOUT) + # the recompute runs once the burst of Broadcasts is over + qtbot.waitUntil( + lambda: _row_items(gui, "q03.IF")[3].data(GUTTER_ROLE) == ["qubit"], + timeout=BROADCAST_TIMEOUT, + ) for item in _row_items(gui, "q03.IF"): assert item.data(QtCore.Qt.ItemDataRole.BackgroundRole) in tint assert _row_items(gui, "q03.IF")[3].data(GUTTER_ROLE) == ["qubit"] From 76b40f496b990f560a7c211ea5f528d00d2af521 Mon Sep 17 00:00:00 2001 From: Marcos Frenkel Date: Tue, 20 Oct 2026 17:52:11 -0500 Subject: [PATCH 05/10] Proxy: keep the units of known parameters current on update update() only added parameters it did not have yet, so a unit the Server changed in place (a Type's unit edit propagates to its Instances) stayed stale on the proxy; the Parameter Manager GUI then saw the Instance's units disagree with its Type and stopped counting it as an Instance. Co-Authored-By: Claude Opus 5.5 --- src/instrumentserver/client/proxy.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/instrumentserver/client/proxy.py b/src/instrumentserver/client/proxy.py index 1a65a17..bc03139 100644 --- a/src/instrumentserver/client/proxy.py +++ b/src/instrumentserver/client/proxy.py @@ -400,6 +400,10 @@ def _getProxyParameters(self) -> None: bluePrint=pbp, setpoints_instrument=self, ) + else: + # the Server can change a unit in place (a Type's unit edit + # propagates to its Instances), so keep it current + self.parameters[pn].unit = p.unit delKeys = [] for pn in self.parameters.keys(): From 2090cc29853f0d823c4b1fd64821b1633c608166 Mon Sep 17 00:00:00 2001 From: Marcos Frenkel Date: Tue, 20 Oct 2026 17:52:12 -0500 Subject: [PATCH 06/10] Parameter Manager GUI: global value column in the Types tab, drop Type Lock re-target - The Types tab's entries pane reads parameter | unit | default | global | locked to global. The global column shows and sets the value every locked Instance follows: the Type Lock's Target, or the entry's _globals.. parameter while unlocked. Default and global boxes are narrower. - ParameterManager.ensure_global_targets() creates the Globals parameter of every Type entry that has none; the GUI calls it so every entry has a global value to edit. set_type_parameter_unit also updates the entry's Globals parameter, so a later Type Lock does not hit a unit conflict. - Remove the Type Lock re-target button, its arm-strip path and the rank_lock_targets arm_rel option only it used. Co-Authored-By: Claude Opus 5.5 --- .../gui/parameter_manager/logic.py | 19 +-- .../gui/parameter_manager/panels.py | 97 ++++++++------ .../gui/parameter_manager/widget.py | 122 ++++++++++-------- src/instrumentserver/params.py | 18 +++ test/pytest/test_pm_gui.py | 96 ++------------ 5 files changed, 160 insertions(+), 192 deletions(-) diff --git a/src/instrumentserver/gui/parameter_manager/logic.py b/src/instrumentserver/gui/parameter_manager/logic.py index a16be1a..c0a439f 100644 --- a/src/instrumentserver/gui/parameter_manager/logic.py +++ b/src/instrumentserver/gui/parameter_manager/logic.py @@ -408,16 +408,12 @@ def rank_lock_targets( follower: str, candidates: Iterable[str], claims: Mapping[str, Claim], - arm_rel: Optional[str] = None, ) -> List[str]: """The arm strip's Target candidates in the mock's completer order. ``arm_rel`` is the Follower's path relative to its Instance (the part behind the Claiming Type's Instance path), or ``None`` when the - Follower is claimed by no Type; an explicit ``arm_rel`` argument - overrides it, which the Types tab's Type Lock re-target (plan task - 5.5) uses to rank for a Type's entry path — there is no claimed - Follower and so nothing to exclude. Rank 0: the candidate's own + Follower is claimed by no Type. Rank 0: the candidate's own relative path equals ``arm_rel`` (the same leaf on a sibling Instance, the mock's first pick). Rank 1: ``.`` occurs in the candidate (a submodule on the way). Rank 2: everything else. Equal ranks order @@ -425,13 +421,12 @@ def rank_lock_targets( not filtered here: the Server refuses them and the arm strip shows its error text. """ - if arm_rel is None: - follower_claim = claims.get(follower) - arm_rel = ( - follower[len(follower_claim.instance) + 1 :] - if follower_claim is not None - else None - ) + follower_claim = claims.get(follower) + arm_rel = ( + follower[len(follower_claim.instance) + 1 :] + if follower_claim is not None + else None + ) def own_rel(candidate: str) -> Optional[str]: claim = claims.get(candidate) diff --git a/src/instrumentserver/gui/parameter_manager/panels.py b/src/instrumentserver/gui/parameter_manager/panels.py index 6d87576..4d6d69f 100644 --- a/src/instrumentserver/gui/parameter_manager/panels.py +++ b/src/instrumentserver/gui/parameter_manager/panels.py @@ -592,11 +592,12 @@ def _build_row_widgets( # ----------------- Types tab ---------------------------------------------------------- -#: Fixed pixel widths of the entries pane's unit, "locked to" and default -#: columns (the mock's 60/200/252 trio). +#: Pixel widths of the entries pane's unit, default, global and "locked to +#: global" columns. ENTRIES_UNIT_WIDTH = 60 ENTRIES_LOCK_WIDTH = 210 -ENTRIES_DEFAULT_WIDTH = 250 +ENTRIES_DEFAULT_WIDTH = 170 +ENTRIES_GLOBAL_WIDTH = 110 #: Fixed pixel widths of the instances pane's parameter-count, "also" and #: button columns (the mock's 150/160/110 trio). @@ -613,10 +614,12 @@ class TypesPane(QtWidgets.QWidget): effective parameters, each row tinted with the Type's colour — and the New type strip. Right, above: the entries of the selected Type as a tree. Own entries carry an editable default (Return or the set button - commits), a Remove button and the Type Lock toggle in the "locked to" - column, with a re-target button and the Target's path while locked. - Entries from Nested Types render read-only with "defined by ". - Submodule rows show ``type: `` in the "locked to" column and, for + commits), a Remove button, the global value in the "global" column — + the Type Lock's Target, or the entry's Globals parameter while + unlocked (Return sets it) — and the Type Lock toggle in the "locked + to global" column, with the Target's path while locked. Entries from Nested Types render read-only with "defined by + ". Submodule rows show ``type: `` in the "locked to global" + column and, for the selected Type's own Nested Types, a Remove button. Beneath the tree run the "Add to type" and "Nested type" strips and a note line. Right, below: the Instances of the selected Type — name, parameter @@ -626,7 +629,7 @@ class TypesPane(QtWidgets.QWidget): The pane never talks to the Server: every action is emitted as a signal — ``addTypeRequested``, ``addEntryRequested``, ``removeEntryRequested``, ``setDefaultRequested``, - ``toggleTypeLockRequested``, ``retargetTypeLockRequested``, + ``toggleTypeLockRequested``, ``addNestedRequested``, ``removeNestedRequested``, ``addInstanceRequested`` and ``showInstanceRequested`` — and :class:`.ParameterManagerGui`, which owns the pane, performs it and @@ -669,17 +672,17 @@ class TypesPane(QtWidgets.QWidget): #: the Type's name, the entry path and the editor's text. setUnitRequested = QtCore.Signal(str, str, str) + #: Signal(str, str) + #: Emitted when the user commits a locked entry's global value editor + #: (Return): the Target's path and the editor's text. + setGlobalRequested = QtCore.Signal(str, str) + #: Signal(str, str) #: Emitted when the user presses a Type Lock toggle: the Type's name #: and the entry path. The GUI locks or unlocks from the entry's #: stored Target. toggleTypeLockRequested = QtCore.Signal(str, str) - #: Signal(str, str) - #: Emitted when the user presses a locked entry's re-target button: - #: the Type's name and the entry path. - retargetTypeLockRequested = QtCore.Signal(str, str) - #: Signal(str, str, str) #: Emitted when the user presses "Add nested type": the Type's name, #: the submodule and the Nested Type's name. @@ -722,6 +725,11 @@ def __init__( # buttons of the instance rows, keyed by instance path self.entryWidgets: Dict[str, Dict[str, Any]] = {} self.showButtons: Dict[str, QtWidgets.QPushButton] = {} + # the values of the selected Type's Type Lock Targets, keyed by + # Target path (set by the GUI before a rebuild), and the global + # value editors showing them + self.globalValues: Dict[str, Any] = {} + self.globalEditors: Dict[str, QtWidgets.QLineEdit] = {} layout = QtWidgets.QVBoxLayout(self) layout.setContentsMargins(0, 0, 0, 0) @@ -785,9 +793,9 @@ def __init__( entriesLayout.setContentsMargins(0, 0, 0, 0) self.entriesLabel = QtWidgets.QLabel(entriesPane) - self.entriesModel = QtGui.QStandardItemModel(0, 4, self) + self.entriesModel = QtGui.QStandardItemModel(0, 5, self) self.entriesModel.setHorizontalHeaderLabels( - ["parameter", "unit", "locked to", "default"] + ["parameter", "unit", "default", "global", "locked to global"] ) self.entriesView = QtWidgets.QTreeView(entriesPane) self.entriesView.setModel(self.entriesModel) @@ -800,8 +808,9 @@ def __init__( entriesHeader.setSectionResizeMode(0, QtWidgets.QHeaderView.ResizeMode.Stretch) for column, width in ( (1, ENTRIES_UNIT_WIDTH), - (2, ENTRIES_LOCK_WIDTH), - (3, ENTRIES_DEFAULT_WIDTH), + (2, ENTRIES_DEFAULT_WIDTH), + (3, ENTRIES_GLOBAL_WIDTH), + (4, ENTRIES_LOCK_WIDTH), ): entriesHeader.setSectionResizeMode( column, QtWidgets.QHeaderView.ResizeMode.Interactive @@ -1067,6 +1076,7 @@ def _rebuild_entries( self._clear_index_widgets() self.entriesModel.removeRows(0, self.entriesModel.rowCount()) self.entryWidgets = {} + self.globalEditors = {} blueprint = types.get(selected) if selected is None or blueprint is None: self.entriesView.expandAll() @@ -1086,17 +1096,19 @@ def _rebuild_entries( unit_item = QtGui.QStandardItem("" if row.kind == "submodule" else row.unit) lock_item = QtGui.QStandardItem() default_item = QtGui.QStandardItem() - parent_item.appendRow([name_item, unit_item, lock_item, default_item]) + global_item = QtGui.QStandardItem() + parent_item.appendRow( + [name_item, unit_item, default_item, global_item, lock_item] + ) items_by_path[path] = name_item if colours is not None: - for item in (name_item, unit_item, lock_item, default_item): + for item in (name_item, unit_item, default_item, global_item, lock_item): item.setData(colours["tint"], QtCore.Qt.ItemDataRole.BackgroundRole) entry: Dict[str, Any] = { "editor": None, "set": None, "remove": None, "toggle": None, - "retarget": None, "targetLabel": None, "definedBy": None, "removeNested": None, @@ -1123,6 +1135,26 @@ def _rebuild_entries( self.entriesView.setIndexWidget( self.entriesModel.indexFromItem(unit_item), unit_editor ) + if row.own: + # the value every locked Instance follows: the Type + # Lock's Target, or the entry's Globals parameter while + # unlocked; Return sets it + target = row.target or f"_globals.{selected}.{row.path}" + value = self.globalValues.get(target) + global_editor = QtWidgets.QLineEdit( + "" if value is None else str(value), + self.entriesView.viewport(), + ) + global_editor.setToolTip(f"the value of {target}") + global_editor.returnPressed.connect( + lambda target=target, global_editor=global_editor: ( + self.setGlobalRequested.emit(target, global_editor.text()) + ) + ) + self.entriesView.setIndexWidget( + self.entriesModel.indexFromItem(global_item), global_editor + ) + self.globalEditors[target] = global_editor self.entriesView.expandAll() def _build_submodule_row( @@ -1181,7 +1213,7 @@ def _build_own_entry_cells( entry: Dict[str, Any], ) -> None: # the "locked to" column: the Type Lock toggle, and while the entry - # is locked the re-target button and the Target's relative path + # is locked the Target's relative path locked = row.target is not None lock_container = QtWidgets.QWidget(self.entriesView.viewport()) lock_layout = QtWidgets.QHBoxLayout(lock_container) @@ -1205,21 +1237,6 @@ def _build_own_entry_cells( lock_layout.addWidget(toggle) entry["toggle"] = toggle if locked: - retarget = QtWidgets.QPushButton( - QtGui.QIcon(":/icons/set.svg"), "", parent=lock_container - ) - retarget.setToolTip( - f"lock every Instance of {selected} to another Target — " - "pick one in the parameter tree" - ) - keepSmallHorizontally(retarget) - retarget.pressed.connect( - lambda type_name=selected, path=row.path: ( - self.retargetTypeLockRequested.emit(type_name, path) - ) - ) - lock_layout.addWidget(retarget) - entry["retarget"] = retarget target_label = QtWidgets.QLabel(row.target, parent=lock_container) target_label.setToolTip( f"{row.target} — followed by every Instance of {selected}" @@ -1443,6 +1460,14 @@ def show_entries_error(self, text: str) -> None: self.entriesNote.setStyleSheet("QLabel { color: red }") self.entriesNote.setText(text) + def show_global_value(self, target: str, value: Any) -> None: + """Show ``value`` on the global value editor of the Target at + ``target``, if the entries pane shows one.""" + self.globalValues[target] = value + editor = self.globalEditors.get(target) + if editor is not None: + editor.setText("" if value is None else str(value)) + def show_entries_note(self, text: str) -> None: """Show ``text`` on the entries pane's note in the normal colour (a skipped-Lock warning, for example).""" diff --git a/src/instrumentserver/gui/parameter_manager/widget.py b/src/instrumentserver/gui/parameter_manager/widget.py index c5b2d5c..62d0454 100644 --- a/src/instrumentserver/gui/parameter_manager/widget.py +++ b/src/instrumentserver/gui/parameter_manager/widget.py @@ -7,7 +7,6 @@ Dict, List, Optional, - Tuple, Union, cast, ) @@ -523,17 +522,14 @@ class LocksController(QtCore.QObject): target pick, and the Locks panel. It works on the GUI's widgets and state and wires them in :meth:`connectSignals`. - ``armed_follower`` is the Follower a pick is armed for, and - ``armed_type_lock`` the (Type, entry) pair a Type Lock re-target from - the Types tab (plan task 5.5) is armed for; both are ``None`` while - nothing is armed. + ``armed_follower`` is the Follower a pick is armed for, ``None`` + while nothing is armed. """ def __init__(self, gui: "ParameterManagerGui") -> None: super().__init__(gui) self.gui = gui self.armed_follower: Optional[str] = None - self.armed_type_lock: Optional[Tuple[str, str]] = None def connectSignals(self) -> None: gui = self.gui @@ -566,8 +562,6 @@ def connectSignals(self) -> None: treeSelection = gui.view.selectionModel() assert treeSelection is not None # set together with the model treeSelection.currentChanged.connect(self._on_tree_current_changed) - # the Types tab's Type Lock re-target arms the same strip - gui.typesPane.retargetTypeLockRequested.connect(self.arm_type_lock) gui.shortcutManager.register("toggle_locks", gui.locksAction.toggle, gui) # the Lock shortcuts (plan task 5.6); the tree's two lock actions # carry their key in their tooltips @@ -769,55 +763,13 @@ def arm_lock(self, follower: str) -> None: parameters = self.gui.model.parameter_units() claims = compute_claims(self.gui.state.types, parameters) self.armed_follower = follower - self.armed_type_lock = None self.gui.armStrip.arm(follower, rank_lock_targets(follower, parameters, claims)) - def arm_type_lock(self, type_name: str, path: str) -> None: - """Arm the target picker for the Type Lock of the entry ``path`` - of the Type ``type_name`` (plan task 5.5): the strip shows on the - Parameters tab with the entry named in its label, and the - candidates are every parameter row of the source model ranked - with the entry path as the relative path (the same leaf on any - Instance first). Arming while already armed re-arms for the new - entry.""" - self.armed_type_lock = (type_name, path) - self.armed_follower = None - self.gui.tabs.setCurrentIndex(0) - parameters = self.gui.model.parameter_units() - claims = compute_claims(self.gui.state.types, parameters) - self.gui.armStrip.arm( - f"type {type_name} \u00b7 {path}", - rank_lock_targets("", parameters, claims, arm_rel=path), - ) - def pick_lock_target(self, target: str) -> None: - """Pick ``target`` as the Target of the armed pick — a Follower's - Lock (plan task 5.3) or, while a Type Lock re-target is armed, the - Type Lock declaration with ``target`` as its Target (plan task - 5.5). A refused Lock — a cycle (D7), a self-lock — shows the + """Pick ``target`` as the Target of the armed Follower's Lock (plan + task 5.3). A refused Lock — a cycle (D7), a self-lock — shows the Server's error text on the strip and stays armed so another - target can be picked; a successful Lock disarms the strip, and a - Type Lock declaration names the Instance parameters it skipped - (D17) on the entries pane's note.""" - if self.armed_type_lock is not None: - type_name, path = self.armed_type_lock - try: - skipped = self.gui.instrument.lock_type_parameter( - type_name, path, target=target - ) - except Exception as exc: - self.gui.armStrip.show_error(str(exc)) - else: - if skipped: - self.gui.typesPane.show_entries_note( - f"skipped: {', '.join(skipped)}" - ) - else: - # a clean declaration leaves no stale error or - # skipped note behind (plan task 5.6) - self.gui.typesPane.reset_entries_note() - self.cancel_arm() - return + target can be picked; a successful Lock disarms the strip.""" if self.armed_follower is None: return try: @@ -828,10 +780,8 @@ def pick_lock_target(self, target: str) -> None: self.cancel_arm() def cancel_arm(self) -> None: - """Disarm the target picker without picking anything (either kind - of pick: a Follower's Lock or a Type Lock's re-target).""" + """Disarm the target picker without picking anything.""" self.armed_follower = None - self.armed_type_lock = None self.gui.armStrip.disarm() @QtCore.Slot(QtCore.QModelIndex) @@ -839,7 +789,7 @@ def _on_view_clicked(self, index: QtCore.QModelIndex) -> None: """A row click while the pick is armed chooses that row's parameter as the Target (the mock's rowClick); a submodule click does nothing.""" - if self.armed_follower is None and self.armed_type_lock is None: + if self.armed_follower is None: return source_index = self.gui.proxyModel.mapToSource(index) source_index = source_index.sibling(source_index.row(), 0) @@ -991,6 +941,8 @@ def connectSignals(self) -> None: gui.typesPane.removeEntryRequested.connect(self._on_pane_remove_entry) gui.typesPane.setDefaultRequested.connect(self._on_pane_set_default) gui.typesPane.setUnitRequested.connect(self._on_pane_set_unit) + gui.typesPane.setGlobalRequested.connect(self._on_pane_set_global) + gui.model.itemNewValue.connect(gui.typesPane.show_global_value) gui.typesPane.toggleTypeLockRequested.connect(self._on_pane_toggle_type_lock) gui.typesPane.addNestedRequested.connect(self._on_pane_add_nested) gui.typesPane.removeNestedRequested.connect(self._on_pane_remove_nested) @@ -1075,14 +1027,70 @@ def refresh_types_pane(self) -> None: returns (the Broadcast arrives on top of that; a double rebuild is fine). The pane keeps the selected Type across rebuilds and drops the selection when the Type is gone.""" + self._load_global_values() self.gui.typesPane.rebuild( self.gui.state.types, self.gui.model.parameter_units(), self.gui.typePalette ) + def _load_global_values(self) -> None: + """Read the global value of every Type entry — its Type Lock's + Target, or its Globals parameter — for the pane's global column. + The read broadcasts a ``parameter-call``, which only repaints rows, + so it cannot bring a rebuild back here.""" + values: Dict[str, Any] = {} + refreshed = False + for type_name, blueprint in self.gui.state.types.items(): + for path, entry in blueprint.parameters.items(): + stored = entry.get("target") + target = ( + relative_path(stored, self.gui.instrument.name) + if stored is not None + else f"_globals.{type_name}.{path}" + ) + if target in values: + continue + try: + values[target] = nestedAttributeFromString( + self.gui.instrument, target + )() + except Exception: + values[target] = None + if not refreshed: + # a Target the Proxy does not know yet (created + # just now, or an entry older than its Globals + # parameter): create the missing Globals + # parameters, refresh the Proxy once, read again + refreshed = True + try: + self.gui.instrument.ensure_global_targets() + if hasattr(self.gui.instrument, "update"): + self.gui.instrument.update() + values[target] = nestedAttributeFromString( + self.gui.instrument, target + )() + except Exception: + pass + self.gui.typesPane.globalValues = values + + @QtCore.Slot(str, str) + def _on_pane_set_global(self, target: str, text: str) -> None: + """An entry's committed global value editor (Return): set the + global value, which every locked Instance follows. A refused set + shows the error text on the entries pane's note.""" + try: + nestedAttributeFromString(self.gui.instrument, target)( + parse_default_text(text) + ) + except Exception as exc: + self.gui.typesPane.show_entries_error(str(exc)) + else: + self.gui.typesPane.reset_entries_note() + @QtCore.Slot(str) def _on_pane_type_selected(self, name: str) -> None: """The Types pane's selected Type changed: re-render the entries and Instances panes for it.""" + self._load_global_values() self.gui.typesPane.refresh_selected_panes( self.gui.state.types, self.gui.model.parameter_units(), self.gui.typePalette ) diff --git a/src/instrumentserver/params.py b/src/instrumentserver/params.py index 36ab13a..baf34ed 100644 --- a/src/instrumentserver/params.py +++ b/src/instrumentserver/params.py @@ -1654,6 +1654,9 @@ def set_type_parameter_unit(self, type_name: str, path: str, unit: str) -> None: propagated.add(full) if self.has_param(full): cast(Parameter, self.parameter(full)).unit = unit + global_path = f"_globals.{type_name}.{path}" + if self.has_param(global_path): + cast(Parameter, self.parameter(global_path)).unit = unit for name in affected: self._broadcast_type_update(name) @@ -2042,6 +2045,21 @@ def _ensure_global_target(self, type_name: str, path: str) -> str: self._broadcast_parameter_creation(global_path, entry.default, entry.unit) return global_path + def ensure_global_targets(self) -> List[str]: + """Create the Globals parameter ``_globals..`` of every + Type entry that has none yet, with the entry's default value and + unit, and return the created paths; the GUI calls it so every + entry has a global value to edit. Raises like + :meth:`_ensure_global_target`.""" + created: List[str] = [] + for type_name, definition in self._types.items(): + for path in definition.parameters: + global_path = f"_globals.{type_name}.{path}" + if not self.has_param(global_path): + self._ensure_global_target(type_name, path) + created.append(global_path) + return created + # ------------------------------------------------------------------ # Type Locks (plan decision D17, task 3.2) # diff --git a/test/pytest/test_pm_gui.py b/test/pytest/test_pm_gui.py index d97b576..beb76d7 100644 --- a/test/pytest/test_pm_gui.py +++ b/test/pytest/test_pm_gui.py @@ -2069,7 +2069,9 @@ def test_the_type_lock_rows_lock_all_and_remove_rule( timeout=BROADCAST_TIMEOUT, ) assert pm.get_lock("dq01.IF").target == f"{PM_NAME}.tshared" - assert not any(path.startswith("_globals") for path in pm.list()) + # the Types pane creates every entry's Globals parameter for its + # global column, so _globals.dqubit.IF exists; the rule and the + # Lock above still name the stored Target finally: gui.model.stopListener() @@ -2432,7 +2434,7 @@ def _type_list_row(gui, name): def _entry_row_items(gui, path): - """The four items of the entries-pane row ``path`` (walked segment by + """The five items of the entries-pane row ``path`` (walked segment by segment through the tree), or ``None`` while the row does not exist.""" item = gui.typesPane.entriesModel.invisibleRootItem() @@ -2449,8 +2451,8 @@ def _entry_row_items(gui, path): parent = item.parent() if parent is None: model = gui.typesPane.entriesModel - return [model.item(item.row(), column) for column in range(4)] - return [parent.child(item.row(), column) for column in range(4)] + return [model.item(item.row(), column) for column in range(5)] + return [parent.child(item.row(), column) for column in range(5)] def _instance_row_items(gui, name): @@ -2645,21 +2647,6 @@ def test_parse_default_text(): assert parse_default_text("'xy'") == "xy" -def test_rank_lock_targets_ranks_for_a_type_lock_arm(): - """With an explicit ``arm_rel`` — the Types tab's Type Lock re-target - — the entry path ranks the candidates: the same leaf on any Instance - first, then submodules on the way, everything else last. There is no - Follower to exclude.""" - claims = { - "q01.IF": Claim(type="qubit", instance="q01", stack=["qubit"]), - "q02.IF": Claim(type="qubit", instance="q02", stack=["qubit"]), - } - ranked = rank_lock_targets( - "", ["other.x", "q02.IF", "q05.IF.gain", "q01.IF"], claims, arm_rel="IF" - ) - assert ranked == ["q01.IF", "q02.IF", "q05.IF.gain", "other.x"] - - def test_a_creation_from_a_second_client_under_an_existing_submodule_appears( qtbot, pm, second_client, server_port ): @@ -3012,7 +2999,7 @@ def test_the_types_tab_edits_entries_and_nested_types( ), timeout=BROADCAST_TIMEOUT, ) - assert _entry_row_items(gui, "readout")[2].text() == "type: readout" + assert _entry_row_items(gui, "readout")[4].text() == "type: readout" defined_by = gui.typesPane.entryWidgets["readout.bw"]["definedBy"] assert defined_by.text() == "defined by readout" assert defined_by.toolTip() == "defined by readout — change the default there" @@ -3031,14 +3018,12 @@ def test_the_types_tab_edits_entries_and_nested_types( gui.model.stopListener() -def test_the_types_tab_type_locks_toggle_and_retarget( +def test_the_types_tab_type_lock_toggle( qtbot, pm, second_client, server_port ): """The Type Lock toggle puts the Globals default Target on the entry, locks the Instance parameter and shows the ``_globals`` row in the - tree; toggling again removes only the rule (D17) and the Locks stay. - The re-target button arms the picker on the Parameters tab and the - picked row becomes the entry's Target.""" + tree; toggling again removes only the rule (D17) and the Locks stay.""" second_pm = _second_parameter_manager(second_client) gui = _make_gui(qtbot, pm, server_port) @@ -3087,55 +3072,6 @@ def _state_target(): ) assert pm.get_lock("q10.IF") is not None - # toggle on once more: the re-target button needs a locked entry - gui.typesPane.entryWidgets["IF"]["toggle"].click() - qtbot.waitUntil( - lambda: ( - _state_target() == globals_target - and pm.get_type("qubit").parameters["IF"]["target"] == globals_target - ), - timeout=BROADCAST_TIMEOUT, - ) - - # re-target through the arm strip, with a root parameter created - # first - pm.add_parameter("tshared", initial_value=0.0, unit="Hz") - qtbot.waitUntil(lambda: _row_exists(gui, "tshared"), timeout=BROADCAST_TIMEOUT) - qtbot.waitUntil( - lambda: ( - gui.typesPane.entryWidgets.get("IF", {}).get("retarget") is not None - ), - timeout=BROADCAST_TIMEOUT, - ) - gui.typesPane.entryWidgets["IF"]["retarget"].click() - assert gui.tabs.currentIndex() == 0 - assert gui.armStrip.label.text() == "Target for type qubit · IF" - assert gui.locksController.armed_type_lock == ("qubit", "IF") - assert gui.locksController.armed_follower is None - - # a Target the Server refuses: the error text on the strip, which - # stays armed, and the entry's Target unchanged on the Server - gui.locksController.pick_lock_target("no.such.path") - qtbot.waitUntil( - lambda: "no.such.path" in gui.armStrip.errorLabel.text(), - timeout=BROADCAST_TIMEOUT, - ) - assert not gui.armStrip.errorLabel.isHidden() - assert not gui.armStrip.isHidden() - assert gui.locksController.armed_type_lock == ("qubit", "IF") - assert gui.locksController.armed_follower is None - assert pm.get_type("qubit").parameters["IF"]["target"] == globals_target - - gui.locksController.pick_lock_target("tshared") - qtbot.waitUntil( - lambda: ( - pm.get_type("qubit").parameters["IF"]["target"] == f"{PM_NAME}.tshared" - ), - timeout=BROADCAST_TIMEOUT, - ) - assert gui.armStrip.isHidden() - assert gui.locksController.armed_type_lock is None - assert gui.locksController.armed_follower is None finally: gui.model.stopListener() @@ -3175,20 +3111,6 @@ def test_the_types_tab_names_skipped_locks_on_the_note( ) assert pm.get_lock("q10.IF").target == f"{PM_NAME}.tshared" - # a clean Type Lock re-target with nothing skipped resets the - # note (plan task 5.6); re-targeting to the Follower's own Target - # skips nothing - gui.locksController.arm_type_lock("qubit", "IF") - assert gui.locksController.armed_type_lock == ("qubit", "IF") - gui.locksController.pick_lock_target("tshared") - qtbot.waitUntil( - lambda: ( - pm.get_type("qubit").parameters["IF"]["target"] == f"{PM_NAME}.tshared" - ), - timeout=BROADCAST_TIMEOUT, - ) - assert gui.typesPane.entriesNote.text() == "" - assert gui.locksController.armed_type_lock is None finally: gui.model.stopListener() From fac59135e98c514fb45fd0f4abbcbfa6af5e0bdd Mon Sep 17 00:00:00 2001 From: Marcos Frenkel Date: Tue, 20 Oct 2026 18:09:55 -0500 Subject: [PATCH 07/10] Parameter Manager GUI: clean up globals on removal, set button for global values - Removing a Type, or one of its entries, also removes its Globals parameters (_globals. or _globals..). The Server drops every Lock on a removed parameter first, so the Instance parameters locked to them go back to their own values. Instance, Type and entry removal share one helper that calls the Server directly and refreshes the proxy once. - The global value cell gets the parameter rows' set button: an edit turns it orange, Return or the button sets the value and turns it back. An uncommitted edit survives pane rebuilds and incoming values. Co-Authored-By: Claude Opus 5.5 --- .../gui/parameter_manager/panels.py | 71 ++++++++++++++----- .../gui/parameter_manager/widget.py | 60 +++++++++++----- 2 files changed, 96 insertions(+), 35 deletions(-) diff --git a/src/instrumentserver/gui/parameter_manager/panels.py b/src/instrumentserver/gui/parameter_manager/panels.py index 4d6d69f..7f7362e 100644 --- a/src/instrumentserver/gui/parameter_manager/panels.py +++ b/src/instrumentserver/gui/parameter_manager/panels.py @@ -9,6 +9,7 @@ List, Mapping, Optional, + Tuple, cast, ) @@ -18,7 +19,7 @@ PMTypeBluePrint, ) from .. import keepSmallHorizontally -from ..parameters import ParameterWidget +from ..parameters import ParameterWidget, SetButton from .logic import ( GUTTER_ROLE, GUTTER_WIDTH, @@ -597,7 +598,7 @@ def _build_row_widgets( ENTRIES_UNIT_WIDTH = 60 ENTRIES_LOCK_WIDTH = 210 ENTRIES_DEFAULT_WIDTH = 170 -ENTRIES_GLOBAL_WIDTH = 110 +ENTRIES_GLOBAL_WIDTH = 140 #: Fixed pixel widths of the instances pane's parameter-count, "also" and #: button columns (the mock's 150/160/110 trio). @@ -725,11 +726,13 @@ def __init__( # buttons of the instance rows, keyed by instance path self.entryWidgets: Dict[str, Dict[str, Any]] = {} self.showButtons: Dict[str, QtWidgets.QPushButton] = {} - # the values of the selected Type's Type Lock Targets, keyed by - # Target path (set by the GUI before a rebuild), and the global - # value editors showing them + # the global values, keyed by Target path (set by the GUI before a + # rebuild); the (editor, set button) pairs showing them; and the + # edited texts not committed yet, which survive rebuilds and + # incoming values self.globalValues: Dict[str, Any] = {} - self.globalEditors: Dict[str, QtWidgets.QLineEdit] = {} + self.globalEditors: Dict[str, Tuple[QtWidgets.QLineEdit, SetButton]] = {} + self.pendingGlobals: Dict[str, str] = {} layout = QtWidgets.QVBoxLayout(self) layout.setContentsMargins(0, 0, 0, 0) @@ -1138,23 +1141,43 @@ def _rebuild_entries( if row.own: # the value every locked Instance follows: the Type # Lock's Target, or the entry's Globals parameter while - # unlocked; Return sets it + # unlocked. Like a parameter row: an edit turns the set + # button orange, and Return or the button sets it target = row.target or f"_globals.{selected}.{row.path}" value = self.globalValues.get(target) + global_cell = QtWidgets.QWidget(self.entriesView.viewport()) + global_layout = QtWidgets.QHBoxLayout(global_cell) + global_layout.setContentsMargins(0, 0, 0, 0) global_editor = QtWidgets.QLineEdit( - "" if value is None else str(value), - self.entriesView.viewport(), + self.pendingGlobals.get( + target, "" if value is None else str(value) + ), + global_cell, ) global_editor.setToolTip(f"the value of {target}") - global_editor.returnPressed.connect( + global_set = SetButton( + QtGui.QIcon(":/icons/set.svg"), "", parent=global_cell + ) + keepSmallHorizontally(global_set) + global_set.setPending(target in self.pendingGlobals) + global_editor.textEdited.connect( + lambda text, target=target, global_set=global_set: ( + self._on_global_edited(target, text, global_set) + ) + ) + commit = ( lambda target=target, global_editor=global_editor: ( self.setGlobalRequested.emit(target, global_editor.text()) ) ) + global_editor.returnPressed.connect(commit) + global_set.pressed.connect(commit) + global_layout.addWidget(global_editor, 1) + global_layout.addWidget(global_set) self.entriesView.setIndexWidget( - self.entriesModel.indexFromItem(global_item), global_editor + self.entriesModel.indexFromItem(global_item), global_cell ) - self.globalEditors[target] = global_editor + self.globalEditors[target] = (global_editor, global_set) self.entriesView.expandAll() def _build_submodule_row( @@ -1460,13 +1483,29 @@ def show_entries_error(self, text: str) -> None: self.entriesNote.setStyleSheet("QLabel { color: red }") self.entriesNote.setText(text) - def show_global_value(self, target: str, value: Any) -> None: + def _on_global_edited(self, target: str, text: str, set_button: SetButton) -> None: + """Remember a global value edit until it is committed, and turn + its set button orange.""" + self.pendingGlobals[target] = text + set_button.setPending(True) + + def show_global_value( + self, target: str, value: Any, committed: bool = False + ) -> None: """Show ``value`` on the global value editor of the Target at - ``target``, if the entries pane shows one.""" + ``target``, if the entries pane shows one. An edit not committed + yet keeps its text unless ``committed`` — the value was just set + from it — which also turns its set button back.""" self.globalValues[target] = value - editor = self.globalEditors.get(target) - if editor is not None: + if committed: + self.pendingGlobals.pop(target, None) + elif target in self.pendingGlobals: + return + widgets = self.globalEditors.get(target) + if widgets is not None: + editor, set_button = widgets editor.setText("" if value is None else str(value)) + set_button.setPending(False) def show_entries_note(self, text: str) -> None: """Show ``text`` on the entries pane's note in the normal colour diff --git a/src/instrumentserver/gui/parameter_manager/widget.py b/src/instrumentserver/gui/parameter_manager/widget.py index 62d0454..5e41849 100644 --- a/src/instrumentserver/gui/parameter_manager/widget.py +++ b/src/instrumentserver/gui/parameter_manager/widget.py @@ -1077,14 +1077,14 @@ def _on_pane_set_global(self, target: str, text: str) -> None: """An entry's committed global value editor (Return): set the global value, which every locked Instance follows. A refused set shows the error text on the entries pane's note.""" + value = parse_default_text(text) try: - nestedAttributeFromString(self.gui.instrument, target)( - parse_default_text(text) - ) + nestedAttributeFromString(self.gui.instrument, target)(value) except Exception as exc: self.gui.typesPane.show_entries_error(str(exc)) else: self.gui.typesPane.reset_entries_note() + self.gui.typesPane.show_global_value(target, value, committed=True) @QtCore.Slot(str) def _on_pane_type_selected(self, name: str) -> None: @@ -1114,11 +1114,13 @@ def _on_pane_add_type(self, name: str) -> None: @QtCore.Slot(str) def _on_pane_remove_type(self, name: str) -> None: """The Remove type button: remove the selected Type (D13) — the - Instances keep their parameters. A refused removal (the Type is - nested in another) shows the Server's error text on the strip's - note.""" + Instances keep their parameters — and its Globals parameters under + ``_globals.``; the Instance parameters locked to them are + unlocked first. A refused removal (the Type is nested in another) + shows the Server's error text on the strip's note.""" try: self.gui.instrument.remove_type(name) + self._remove_parameters(self._globals_under(f"_globals.{name}")) except Exception as exc: self.gui.typesPane.show_type_error(str(exc)) else: @@ -1148,10 +1150,15 @@ def _on_pane_add_entry( @QtCore.Slot(str, str) def _on_pane_remove_entry(self, type_name: str, path: str) -> None: """An own entry's Remove button: remove the entry from the Type - only (D13) — the Instances keep the parameter. A refused removal - shows the Server's error text on the entries pane's note.""" + (D13) — the Instances keep the parameter — and its Globals + parameter; the Instance parameters locked to it are unlocked + first. A refused removal shows the Server's error text on the + entries pane's note.""" try: self.gui.instrument.remove_type_parameter(type_name, path) + self._remove_parameters( + self._globals_under(f"_globals.{type_name}.{path}") + ) except Exception as exc: self.gui.typesPane.show_entries_error(str(exc)) else: @@ -1274,24 +1281,39 @@ def _on_pane_remove_instance(self, instance: str) -> None: ) if answer != QtWidgets.QMessageBox.StandardButton.Yes: return - # call the Server directly and refresh the proxy once: the proxy's - # remove_parameter refetches the whole blueprint after every call - instrument = self.gui.instrument - cli = getattr(instrument, "cli", None) try: - for path in paths: - if cli is None: - instrument.remove_parameter(path) - else: - cli.call(f"{instrument.remotePath}.remove_parameter", path) - if cli is not None: - instrument.update() + self._remove_parameters(paths) except Exception as exc: self.gui.typesPane.show_instances_error(str(exc)) else: self.gui.typesPane.reset_instances_note() self.refresh_types_pane() + def _remove_parameters(self, paths: List[str]) -> None: + """Remove the parameters at ``paths``; the emptied Parameter Groups + go with them, and every Lock on one of them is dropped, so its + Follower answers with its own value again. The Server is called + directly and the proxy refreshed once: the proxy's + remove_parameter refetches the whole blueprint after every call.""" + instrument = self.gui.instrument + cli = getattr(instrument, "cli", None) + for path in paths: + if cli is None: + instrument.remove_parameter(path) + else: + cli.call(f"{instrument.remotePath}.remove_parameter", path) + if cli is not None and paths: + instrument.update() + + def _globals_under(self, prefix: str) -> List[str]: + """The Globals parameters at or under ``prefix`` — a Type's + ``_globals.`` or an entry's ``_globals..``.""" + return [ + path + for path in self.gui.model.parameter_units() + if path == prefix or path.startswith(f"{prefix}.") + ] + @QtCore.Slot(str, str) def _on_pane_show_instance(self, type_name: str, instance: str) -> None: """An instance row's Show button: switch to the Parameters tab, From 6d474be52047f85e65f75798ccf0401ba43bf9f2 Mon Sep 17 00:00:00 2001 From: Marcos Frenkel Date: Tue, 20 Oct 2026 18:14:05 -0500 Subject: [PATCH 08/10] Parameter Manager GUI: set button for Type entry defaults turns orange on edit The default editor in the Types tab gets the parameter rows' set button: an edit turns it orange, Return or the button sets the default and the next rebuild shows it plain again. An uncommitted edit survives pane rebuilds. Also apply ruff format to the two GUI files. Co-Authored-By: Claude Opus 5.5 --- .../gui/parameter_manager/panels.py | 51 ++++++++++++++----- .../gui/parameter_manager/widget.py | 5 +- 2 files changed, 40 insertions(+), 16 deletions(-) diff --git a/src/instrumentserver/gui/parameter_manager/panels.py b/src/instrumentserver/gui/parameter_manager/panels.py index 7f7362e..385cd49 100644 --- a/src/instrumentserver/gui/parameter_manager/panels.py +++ b/src/instrumentserver/gui/parameter_manager/panels.py @@ -733,6 +733,8 @@ def __init__( self.globalValues: Dict[str, Any] = {} self.globalEditors: Dict[str, Tuple[QtWidgets.QLineEdit, SetButton]] = {} self.pendingGlobals: Dict[str, str] = {} + # the same for the default editors, keyed by (Type, entry path) + self.pendingDefaults: Dict[Tuple[str, str], str] = {} layout = QtWidgets.QVBoxLayout(self) layout.setContentsMargins(0, 0, 0, 0) @@ -1105,7 +1107,13 @@ def _rebuild_entries( ) items_by_path[path] = name_item if colours is not None: - for item in (name_item, unit_item, default_item, global_item, lock_item): + for item in ( + name_item, + unit_item, + default_item, + global_item, + lock_item, + ): item.setData(colours["tint"], QtCore.Qt.ItemDataRole.BackgroundRole) entry: Dict[str, Any] = { "editor": None, @@ -1162,13 +1170,13 @@ def _rebuild_entries( global_set.setPending(target in self.pendingGlobals) global_editor.textEdited.connect( lambda text, target=target, global_set=global_set: ( - self._on_global_edited(target, text, global_set) + self._on_edited( + self.pendingGlobals, target, text, global_set + ) ) ) - commit = ( - lambda target=target, global_editor=global_editor: ( - self.setGlobalRequested.emit(target, global_editor.text()) - ) + commit = lambda target=target, global_editor=global_editor: ( + self.setGlobalRequested.emit(target, global_editor.text()) ) global_editor.returnPressed.connect(commit) global_set.pressed.connect(commit) @@ -1271,17 +1279,27 @@ def _build_own_entry_cells( ) # the default column: the editable default with its set button and - # the entry's Remove button + # the entry's Remove button; like a parameter row, an edit turns the + # set button orange until Return or the button sets it editor_container = QtWidgets.QWidget(self.entriesView.viewport()) editor_layout = QtWidgets.QHBoxLayout(editor_container) editor_layout.setContentsMargins(0, 0, 0, 0) editor = QtWidgets.QLineEdit(editor_container) - editor.setText("" if row.default is None else str(row.default)) + key = (selected, row.path) + editor.setText( + self.pendingDefaults.get( + key, "" if row.default is None else str(row.default) + ) + ) editor.setPlaceholderText("no default") - set_button = QtWidgets.QPushButton( + set_button = SetButton( QtGui.QIcon(":/icons/set.svg"), "", parent=editor_container ) keepSmallHorizontally(set_button) + set_button.setPending(key in self.pendingDefaults) + editor.textEdited.connect( + lambda text: self._on_edited(self.pendingDefaults, key, text, set_button) + ) set_button.pressed.connect( lambda: self.setDefaultRequested.emit(selected, row.path, editor.text()) ) @@ -1483,12 +1501,19 @@ def show_entries_error(self, text: str) -> None: self.entriesNote.setStyleSheet("QLabel { color: red }") self.entriesNote.setText(text) - def _on_global_edited(self, target: str, text: str, set_button: SetButton) -> None: - """Remember a global value edit until it is committed, and turn - its set button orange.""" - self.pendingGlobals[target] = text + def _on_edited( + self, pending: Dict[Any, str], key: Any, text: str, set_button: SetButton + ) -> None: + """Remember an edit of a global value or default until it is + committed, and turn its set button orange.""" + pending[key] = text set_button.setPending(True) + def default_committed(self, type_name: str, path: str) -> None: + """Forget the committed default edit of the entry ``path``: the + next rebuild shows the stored default with a plain set button.""" + self.pendingDefaults.pop((type_name, path), None) + def show_global_value( self, target: str, value: Any, committed: bool = False ) -> None: diff --git a/src/instrumentserver/gui/parameter_manager/widget.py b/src/instrumentserver/gui/parameter_manager/widget.py index 5e41849..7c26f8f 100644 --- a/src/instrumentserver/gui/parameter_manager/widget.py +++ b/src/instrumentserver/gui/parameter_manager/widget.py @@ -1156,9 +1156,7 @@ def _on_pane_remove_entry(self, type_name: str, path: str) -> None: entries pane's note.""" try: self.gui.instrument.remove_type_parameter(type_name, path) - self._remove_parameters( - self._globals_under(f"_globals.{type_name}.{path}") - ) + self._remove_parameters(self._globals_under(f"_globals.{type_name}.{path}")) except Exception as exc: self.gui.typesPane.show_entries_error(str(exc)) else: @@ -1179,6 +1177,7 @@ def _on_pane_set_default(self, type_name: str, path: str, text: str) -> None: self.gui.typesPane.show_entries_error(str(exc)) else: self.gui.typesPane.reset_entries_note() + self.gui.typesPane.default_committed(type_name, path) self.refresh_types_pane() @QtCore.Slot(str, str, str) From 8b8d502578dfc44fb9e45a2093e80246a5b7ae91 Mon Sep 17 00:00:00 2001 From: Marcos Frenkel Date: Wed, 21 Oct 2026 15:01:08 -0500 Subject: [PATCH 09/10] adding tests for new pm --- test/pytest/test_basic_functionality.py | 22 + test/pytest/test_pm_gui.py | 1045 ++++++++++++++++++++++- test/pytest/test_pm_types.py | 252 ++++++ 3 files changed, 1315 insertions(+), 4 deletions(-) diff --git a/test/pytest/test_basic_functionality.py b/test/pytest/test_basic_functionality.py index f0fabac..957c89a 100644 --- a/test/pytest/test_basic_functionality.py +++ b/test/pytest/test_basic_functionality.py @@ -1,3 +1,5 @@ +import time + from qcodes.math_utils.field_vector import FieldVector @@ -71,3 +73,23 @@ def test_sending_and_receiving_arbitrary_objects(cli, start_server): new_field_vector = FieldVector(101, 102, 103) magnet.set_field(new_field_vector) assert magnet.get_field().is_equal(new_field_vector) + + +def test_the_server_goes_idle_after_answering_requests(cli, start_server): + """Regression: the Server's request loop is woken when a worker has + finished a request, and must drain that wakeup. Left undrained, every + poll returns at once and an idle Server spins a whole core. After a + few requests the process may use only a fraction of an idle second's + CPU time; the Server runs in this process, on its own thread.""" + for _ in range(3): + cli.list_instruments() + time.sleep(0.5) # the last response goes out and the loop settles + + start = time.process_time() + time.sleep(1.0) + used = time.process_time() - start + + assert used < 0.5, ( + f"the process used {used:.2f} s of CPU time in an idle second: " + "the Server's request loop does not go idle" + ) diff --git a/test/pytest/test_pm_gui.py b/test/pytest/test_pm_gui.py index beb76d7..aed31e1 100644 --- a/test/pytest/test_pm_gui.py +++ b/test/pytest/test_pm_gui.py @@ -40,6 +40,23 @@ parameter-update for an unknown row recomputes the tints, stale notes are reset on success, and with no Type selected the Types tab strips are disabled and the pane labels carry no trailing space. + +The Types tab editing tests cover the additions on top of 5.6. Without a +Server: the Remove type button showing only while a Type is selected, +the entries pane's columns (parameter, unit, default, global, locked to +global), the editable unit of own entries, the global value editor every +own entry carries (the Type Lock's Target, or ``_globals..`` +while unlocked) and the default editor, both with the parameter rows' +set button that turns orange on an edit, keeps the edit through rebuilds +and incoming values, and commits on Return or the button; the instance +rows' delete button next to Show; and the removed Type Lock re-target. +Live: removing a Type or an entry also removes its Globals parameters, +unlocking the Instance parameters locked to them while the Instances +keep their parameters; a unit edit keeps the Instances recognised; the +instance delete button asks first, removes the whole Instance with one +proxy refresh and names a refused removal; the GUI creates the Globals +parameter of every entry; the global and default editors set the Server; +and a burst of structural Broadcasts recomputes the tints once. """ import importlib @@ -51,6 +68,8 @@ from instrumentserver import QtCore, QtGui, QtWidgets from instrumentserver.blueprints import ( PARAMETER_CALL, + PARAMETER_CREATION, + PARAMETER_DELETION, PARAMETER_UPDATE, PM_LOCK_UPDATE, PM_TYPE_UPDATE, @@ -91,14 +110,16 @@ LOCK_ROW_ROLE, GutterDelegate, LockArmStrip, + TypesPane, ) from instrumentserver.gui.parameter_manager.widget import ( ModelParameterManager, ParameterManagerGui, ParameterManagerTreeView, ) -from instrumentserver.gui.parameters import ParameterWidget +from instrumentserver.gui.parameters import ParameterWidget, SetButton from instrumentserver.gui.shortcuts import KeyboardShortcutManager +from instrumentserver.helpers import nestedAttributeFromString PM_NAME = "parameter_manager" PM_CLASS = "instrumentserver.params.ParameterManager" @@ -3018,9 +3039,7 @@ def test_the_types_tab_edits_entries_and_nested_types( gui.model.stopListener() -def test_the_types_tab_type_lock_toggle( - qtbot, pm, second_client, server_port -): +def test_the_types_tab_type_lock_toggle(qtbot, pm, second_client, server_port): """The Type Lock toggle puts the Globals default Target on the entry, locks the Instance parameter and shows the ``_globals`` row in the tree; toggling again removes only the rule (D17) and the Locks stay.""" @@ -3573,3 +3592,1021 @@ def test_a_parameter_update_for_an_unknown_row_recomputes_the_tints( assert _row_items(gui, "q03.IF")[3].data(GUTTER_ROLE) == ["qubit"] finally: gui.model.stopListener() + + +# --------------------------------------------------------------------------- +# Types tab editing: Remove type, unit and global value editors, the +# orange set buttons, instance deletion, Globals cleanup and the removed +# Type Lock re-target. +# --------------------------------------------------------------------------- + + +ENTRIES_HEADERS = ["parameter", "unit", "default", "global", "locked to global"] + + +def _pane_blueprint(name, entries, nested=None, nested_entries=None): + """A ``PMTypeBluePrint`` for the pane tests: own ``entries`` as + ``{path: (default, unit, target)}`` (``target`` in the full dotted form + the Server stores, or ``None``), the ``nested`` map, and the entries the + Nested Types bring in as ``{path: (unit, defining Type)}``.""" + parameters = { + path: {"default": default, "unit": unit, "target": target} + for path, (default, unit, target) in entries.items() + } + effective = { + path: {"unit": unit, "from_type": name} + for path, (_default, unit, _target) in entries.items() + } + for path, (unit, from_type) in (nested_entries or {}).items(): + effective[path] = {"unit": unit, "from_type": from_type} + return PMTypeBluePrint( + name=name, parameters=parameters, nested=nested or {}, effective=effective + ) + + +def _make_pane(qtbot, types, selected=None, parameters=None, global_values=None): + """Build a standalone Types pane, rebuilt from ``types`` with + ``selected`` selected, the parameter rows ``parameters`` and the + global values ``global_values``. Returns the pane and a rebuild + callable that rebuilds it from the same state.""" + pane = TypesPane(PM_NAME) + qtbot.addWidget(pane) + palette = TypePalette() + palette.sync(types) + pane.globalValues = dict(global_values or {}) + if selected is not None: + pane.select_type(selected) + + def rebuild(): + pane.rebuild(types, parameters or {}, palette) + + rebuild() + return pane, rebuild + + +def _pane_entry_items(pane, path): + """The five items of the entries-pane row ``path`` of a standalone + pane (walked segment by segment), or ``None`` while it does not + exist.""" + item = pane.entriesModel.invisibleRootItem() + for segment in path.split("."): + found = None + for row in range(item.rowCount()): + child = item.child(row, 0) + if child is not None and child.text() == segment: + found = child + break + if found is None: + return None + item = found + parent = item.parent() + if parent is None: + return [pane.entriesModel.item(item.row(), column) for column in range(5)] + return [parent.child(item.row(), column) for column in range(5)] + + +def _pane_cell_widget(pane, path, column): + """The widget the entries view hosts in ``column`` of the row + ``path``, or ``None``.""" + items = _pane_entry_items(pane, path) + assert items is not None, f"no entries row {path!r}" + return pane.entriesView.indexWidget(pane.entriesModel.indexFromItem(items[column])) + + +def _is_pending(button): + """Whether a set button shows an uncommitted edit (orange).""" + return "orange" in button.styleSheet() + + +def _global_widgets(pane, target): + """The (editor, set button) pair of the global value of ``target``.""" + assert target in pane.globalEditors, f"no global editor for {target!r}" + return pane.globalEditors[target] + + +def _server_value(pm, path): + """The current value of the parameter at ``path``, read through the + Proxy after refreshing it.""" + pm.update() + return nestedAttributeFromString(pm, path)() + + +def _server_unit(pm, path): + """The current unit of the parameter at ``path`` as the refreshed + Proxy reports it.""" + pm.update() + return nestedAttributeFromString(pm, path).unit + + +def _instance_buttons(gui, name): + """The (Show, delete) buttons of the instances-pane row ``name``.""" + items = _instance_row_items(gui, name) + assert items is not None, f"no instance row {name!r}" + cell = gui.typesPane.instancesView.indexWidget( + gui.typesPane.instancesModel.indexFromItem(items[3]) + ) + assert cell is not None, f"the instance row {name!r} hosts no buttons" + buttons = cell.findChildren(QtWidgets.QPushButton) + show = [button for button in buttons if button.text() == "Show"] + delete = [button for button in buttons if button.text() != "Show"] + assert len(show) == 1 and len(delete) == 1, buttons + return show[0], delete[0] + + +def _answer_dialogs(monkeypatch, answer, before=None): + """Answer every ``QMessageBox.question`` with ``answer``, running + ``before`` first (to change the Server while the dialog is open).""" + + def question(*args, **kwargs): + if before is not None: + before() + return answer + + monkeypatch.setattr(QtWidgets.QMessageBox, "question", staticmethod(question)) + + +def test_the_remove_type_button_shows_only_while_a_type_is_selected(qtbot): + """With no Type there is nothing to remove and the Remove type button + is hidden; once a Type is selected it shows next to Add, and pressing + it asks to remove the selected Type.""" + pane, _rebuild = _make_pane(qtbot, {}) + assert pane.selectedType is None + assert pane.removeTypeButton.isHidden() + + types = { + "qubit": _pane_blueprint("qubit", {"IF": (1.0, "Hz", None)}), + "readout": _pane_blueprint("readout", {"bw": (2.0, "Hz", None)}), + } + pane, _rebuild = _make_pane(qtbot, types, selected="readout") + assert pane.selectedType == "readout" + assert not pane.removeTypeButton.isHidden() + + with qtbot.waitSignal(pane.removeTypeRequested, timeout=1000) as blocker: + pane.removeTypeButton.click() + assert blocker.args == ["readout"] + + +def test_the_entries_pane_columns_put_the_global_value_before_its_lock(qtbot): + """The entries pane reads parameter, unit, default, global and locked + to global, left to right; a submodule row names its Nested Type in the + last column.""" + readout = _pane_blueprint("readout", {"bw": (2.0, "Hz", None)}) + qubit = _pane_blueprint( + "qubit", + {"IF": (1.0, "Hz", None)}, + nested={"readout": "readout"}, + nested_entries={"readout.bw": ("Hz", "readout")}, + ) + pane, _rebuild = _make_pane( + qtbot, {"readout": readout, "qubit": qubit}, selected="qubit" + ) + + headers = [ + pane.entriesModel.headerData(column, QtCore.Qt.Orientation.Horizontal) + for column in range(pane.entriesModel.columnCount()) + ] + assert headers == ENTRIES_HEADERS + assert _pane_entry_items(pane, "readout")[4].text() == "type: readout" + + +def test_an_own_entry_unit_is_editable_and_return_requests_the_change(qtbot): + """An own entry's unit cell is an editor holding the unit; Return asks + to set the typed unit. An entry from a Nested Type keeps a read-only + unit — its unit is edited on the Type that defines it.""" + readout = _pane_blueprint("readout", {"bw": (2.0, "Hz", None)}) + qubit = _pane_blueprint( + "qubit", + {"IF": (1.0, "Hz", None)}, + nested={"readout": "readout"}, + nested_entries={"readout.bw": ("Hz", "readout")}, + ) + pane, _rebuild = _make_pane( + qtbot, {"readout": readout, "qubit": qubit}, selected="qubit" + ) + + unit_editor = _pane_cell_widget(pane, "IF", 1) + assert isinstance(unit_editor, QtWidgets.QLineEdit) + assert unit_editor.text() == "Hz" + unit_editor.selectAll() + qtbot.keyClicks(unit_editor, "MHz") + with qtbot.waitSignal(pane.setUnitRequested, timeout=1000) as blocker: + qtbot.keyClick(unit_editor, QtCore.Qt.Key.Key_Return) + assert blocker.args == ["qubit", "IF", "MHz"] + + assert not isinstance(_pane_cell_widget(pane, "readout.bw", 1), QtWidgets.QLineEdit) + assert _pane_entry_items(pane, "readout.bw")[1].text() == "Hz" + + +def test_every_own_entry_carries_a_global_value_editor(qtbot): + """Every own entry shows its global value — the entry's Globals + parameter while unlocked, the Type Lock's Target while locked — in an + editor with a set button, whether or not it is locked; a value of + ``None`` shows an empty editor. Entries from a Nested Type carry no + global editor in the nesting Type.""" + readout = _pane_blueprint("readout", {"bw": (2.0, "Hz", None)}) + qubit = _pane_blueprint( + "qubit", + { + "IF": (1.0, "Hz", None), + "gain": (0.5, "", f"{PM_NAME}._globals.qubit.gain"), + "drive": (3.0, "V", f"{PM_NAME}.tshared"), + "spare": (None, "", None), + }, + nested={"readout": "readout"}, + nested_entries={"readout.bw": ("Hz", "readout")}, + ) + values = { + "_globals.qubit.IF": 5.0, + "_globals.qubit.gain": 0.25, + "tshared": 7, + "_globals.qubit.spare": None, + } + pane, _rebuild = _make_pane( + qtbot, + {"readout": readout, "qubit": qubit}, + selected="qubit", + global_values=values, + ) + + assert set(pane.globalEditors) == set(values) + for target, value in values.items(): + editor, set_button = _global_widgets(pane, target) + assert isinstance(editor, QtWidgets.QLineEdit) + assert isinstance(set_button, SetButton) + assert editor.text() == ("" if value is None else str(value)) + assert not _is_pending(set_button) + # the editors sit in the global column of their rows + for path, target in ( + ("IF", "_globals.qubit.IF"), + ("gain", "_globals.qubit.gain"), + ("drive", "tshared"), + ("spare", "_globals.qubit.spare"), + ): + cell = _pane_cell_widget(pane, path, 3) + assert cell is not None + assert _global_widgets(pane, target)[0] in cell.findChildren( + QtWidgets.QLineEdit + ) + assert _pane_cell_widget(pane, "readout.bw", 3) is None + + +def test_a_global_value_edit_turns_its_set_button_orange_until_committed(qtbot): + """Like a parameter row: typing in a global editor turns its set + button orange; the edit survives a rebuild of the pane and an incoming + value for the same Target; Return and the set button both ask to set + the typed text; a committed value shows the value with a plain button + again.""" + qubit = _pane_blueprint("qubit", {"IF": (1.0, "Hz", None)}) + target = "_globals.qubit.IF" + pane, rebuild = _make_pane( + qtbot, {"qubit": qubit}, selected="qubit", global_values={target: 5.0} + ) + + editor, set_button = _global_widgets(pane, target) + assert not _is_pending(set_button) + editor.selectAll() + qtbot.keyClicks(editor, "6.5") + assert editor.text() == "6.5" + assert _is_pending(set_button) + + # a rebuild keeps the uncommitted edit and its orange button + rebuild() + editor, set_button = _global_widgets(pane, target) + assert editor.text() == "6.5" + assert _is_pending(set_button) + + # an incoming value does not overwrite the uncommitted edit + pane.show_global_value(target, 8.0) + editor, set_button = _global_widgets(pane, target) + assert editor.text() == "6.5" + assert _is_pending(set_button) + + with qtbot.waitSignal(pane.setGlobalRequested, timeout=1000) as blocker: + qtbot.keyClick(editor, QtCore.Qt.Key.Key_Return) + assert blocker.args == [target, "6.5"] + with qtbot.waitSignal(pane.setGlobalRequested, timeout=1000) as blocker: + set_button.click() + assert blocker.args == [target, "6.5"] + + # the value set from the edit is committed: the button turns plain and + # later rebuilds and incoming values show the Server's value again + pane.show_global_value(target, 6.5, committed=True) + editor, set_button = _global_widgets(pane, target) + assert editor.text() == "6.5" + assert not _is_pending(set_button) + pane.show_global_value(target, 9.0) + rebuild() + editor, set_button = _global_widgets(pane, target) + assert editor.text() == "9.0" + assert not _is_pending(set_button) + + +def test_a_default_edit_turns_its_set_button_orange_until_committed(qtbot): + """The default editor behaves like the global one: typing turns its + set button orange, the edit survives a rebuild, Return and the set + button ask to set the typed text, and once committed the next rebuild + shows the stored default with a plain button.""" + qubit = _pane_blueprint("qubit", {"IF": (1.0, "Hz", None)}) + pane, rebuild = _make_pane(qtbot, {"qubit": qubit}, selected="qubit") + + widgets = pane.entryWidgets["IF"] + assert widgets["editor"].text() == "1.0" + assert isinstance(widgets["set"], SetButton) + assert not _is_pending(widgets["set"]) + + widgets["editor"].selectAll() + qtbot.keyClicks(widgets["editor"], "2.5") + assert _is_pending(widgets["set"]) + + rebuild() + widgets = pane.entryWidgets["IF"] + assert widgets["editor"].text() == "2.5" + assert _is_pending(widgets["set"]) + + with qtbot.waitSignal(pane.setDefaultRequested, timeout=1000) as blocker: + qtbot.keyClick(widgets["editor"], QtCore.Qt.Key.Key_Return) + assert blocker.args == ["qubit", "IF", "2.5"] + with qtbot.waitSignal(pane.setDefaultRequested, timeout=1000) as blocker: + widgets["set"].click() + assert blocker.args == ["qubit", "IF", "2.5"] + + pane.default_committed("qubit", "IF") + rebuild() + widgets = pane.entryWidgets["IF"] + assert widgets["editor"].text() == "1.0" + assert not _is_pending(widgets["set"]) + + +def test_an_instance_row_carries_a_delete_button_next_to_show(qtbot): + """Every instance row hosts Show and a delete button; the delete + button asks to delete that Instance.""" + qubit = _pane_blueprint("qubit", {"IF": (1.0, "Hz", None)}) + pane, _rebuild = _make_pane( + qtbot, + {"qubit": qubit}, + selected="qubit", + parameters={"q01.IF": "Hz", "q02.IF": "Hz"}, + ) + + model = pane.instancesModel + for name in ("q01", "q02"): + matches = model.findItems(name, QtCore.Qt.MatchFlag.MatchExactly, 0) + assert matches, f"no instance row {name!r}" + cell = pane.instancesView.indexWidget(model.index(matches[0].row(), 3)) + buttons = cell.findChildren(QtWidgets.QPushButton) + assert [button.text() for button in buttons].count("Show") == 1 + delete = [button for button in buttons if button.text() != "Show"] + assert len(delete) == 1 + with qtbot.waitSignal(pane.removeInstanceRequested, timeout=1000) as blocker: + delete[0].click() + assert blocker.args == [name] + + +def test_a_type_lock_has_no_re_target_button(qtbot): + """The Type Lock re-target is gone: a locked entry's "locked to + global" cell holds the lock toggle and the Target's path, and no other + button; the pane offers no re-target request and the arm ranking takes + no entry path.""" + qubit = _pane_blueprint( + "qubit", {"IF": (1.0, "Hz", f"{PM_NAME}._globals.qubit.IF")} + ) + pane, _rebuild = _make_pane( + qtbot, + {"qubit": qubit}, + selected="qubit", + global_values={"_globals.qubit.IF": 1.0}, + ) + + widgets = pane.entryWidgets["IF"] + assert widgets.get("retarget") is None + lock_cell = _pane_cell_widget(pane, "IF", 4) + assert lock_cell.findChildren(QtWidgets.QPushButton) == [widgets["toggle"]] + labels = [label.text() for label in lock_cell.findChildren(QtWidgets.QLabel)] + assert "_globals.qubit.IF" in labels + assert not hasattr(pane, "retargetTypeLockRequested") + + claims = {"q01.IF": Claim(type="qubit", instance="q01", stack=["qubit"])} + with pytest.raises(TypeError): + rank_lock_targets("", ["q01.IF"], claims, arm_rel="IF") + + +def test_the_remove_type_button_removes_the_type_and_its_globals( + qtbot, pm, second_client, server_port +): + """Remove type removes the selected Type and its Globals parameters; + the Instance keeps its parameters, the Type leaves the list and the + note stays empty. Another Type whose name starts the same keeps its + Globals parameters.""" + second_pm = _second_parameter_manager(second_client) + gui = _make_gui(qtbot, pm, server_port) + try: + _wait_until_broadcasts_arrive(qtbot, gui, second_pm) + second_pm.add_type("qubit2") + second_pm.add_type_parameter("qubit2", "IF", default=9.0, unit="Hz") + _create_type_with_instance(qtbot, gui, pm) + qtbot.waitUntil( + lambda: ( + pm.has_param("_globals.qubit.IF") and pm.has_param("_globals.qubit2.IF") + ), + timeout=BROADCAST_TIMEOUT, + ) + + assert gui.typesPane.selectedType == "qubit" + gui.typesPane.removeTypeButton.click() + qtbot.waitUntil( + lambda: "qubit" not in pm.list_types(), timeout=BROADCAST_TIMEOUT + ) + qtbot.waitUntil( + lambda: not pm.has_param("_globals.qubit.IF"), timeout=BROADCAST_TIMEOUT + ) + qtbot.waitUntil( + lambda: _type_list_row(gui, "qubit") is None, timeout=BROADCAST_TIMEOUT + ) + + assert pm.has_param("q10.IF") + assert pm.has_param("_globals.qubit2.IF") + assert "qubit2" in pm.list_types() + assert gui.typesPane.typeNote.text() == "" + finally: + gui.model.stopListener() + + +def test_removing_a_type_unlocks_the_instances_locked_to_its_globals( + qtbot, pm, second_client, server_port +): + """An Instance parameter locked to a Globals parameter of the removed + Type is unlocked before that parameter goes: it keeps its parameter + and answers with its own value again.""" + second_pm = _second_parameter_manager(second_client) + gui = _make_gui(qtbot, pm, server_port) + try: + _wait_until_broadcasts_arrive(qtbot, gui, second_pm) + _create_type_with_instance(qtbot, gui, pm) + globals_target = f"{PM_NAME}._globals.qubit.IF" + + gui.typesPane.entryWidgets["IF"]["toggle"].click() + qtbot.waitUntil( + lambda: ( + pm.get_lock("q10.IF") + == PMLockBluePrint(target=globals_target, locked=True) + ), + timeout=BROADCAST_TIMEOUT, + ) + second_pm.update() + nestedAttributeFromString(second_pm, "_globals.qubit.IF")(42.0) + assert _server_value(pm, "q10.IF") == 42.0 + + gui.typesPane.removeTypeButton.click() + qtbot.waitUntil( + lambda: not pm.has_param("_globals.qubit.IF"), timeout=BROADCAST_TIMEOUT + ) + assert pm.get_lock("q10.IF") is None + assert pm.list_locks() == {} + assert pm.has_param("q10.IF") + # its own value, kept underneath while it was locked + assert _server_value(pm, "q10.IF") == 1.0 + finally: + gui.model.stopListener() + + +def test_a_refused_type_removal_names_the_reason_and_removes_nothing( + qtbot, pm, second_client, server_port +): + """A Type nested in another cannot be removed: the Server's reason + shows in red on the New type strip's note, and the Type and its + Globals parameters stay.""" + second_pm = _second_parameter_manager(second_client) + second_pm.add_type("readout") + second_pm.add_type_parameter("readout", "bw", default=2.0, unit="Hz") + second_pm.add_type("qubit") + second_pm.add_nested_type("qubit", "readout", "readout") + gui = _make_gui(qtbot, pm, server_port) + try: + _wait_until_broadcasts_arrive(qtbot, gui, second_pm) + gui.tabs.setCurrentIndex(1) + gui.typesPane.select_type("readout") + gui.typesController.refresh_types_pane() + qtbot.waitUntil( + lambda: gui.typesPane.selectedType == "readout", timeout=BROADCAST_TIMEOUT + ) + qtbot.waitUntil( + lambda: pm.has_param("_globals.readout.bw"), timeout=BROADCAST_TIMEOUT + ) + + gui.typesPane.removeTypeButton.click() + qtbot.waitUntil( + lambda: gui.typesPane.typeNote.text() != "", timeout=BROADCAST_TIMEOUT + ) + assert "red" in gui.typesPane.typeNote.styleSheet() + assert "readout" in pm.list_types() + assert pm.has_param("_globals.readout.bw") + finally: + gui.model.stopListener() + + +def test_removing_an_entry_removes_its_global_and_unlocks_the_instances( + qtbot, pm, second_client, server_port +): + """An own entry's Remove button removes the entry and its Globals + parameter; the Instance parameter locked to it is unlocked and stays, + and the other entries keep their Globals parameters.""" + second_pm = _second_parameter_manager(second_client) + gui = _make_gui(qtbot, pm, server_port) + try: + _wait_until_broadcasts_arrive(qtbot, gui, second_pm) + _create_type_with_instance(qtbot, gui, pm) + gui.typesPane.entryNameEdit.setText("gain") + gui.typesPane.entryDefaultEdit.setText("0.5") + gui.typesPane.entryUnitEdit.setText("") + gui.typesPane.addEntryButton.click() + qtbot.waitUntil( + lambda: pm.has_param("_globals.qubit.gain"), timeout=BROADCAST_TIMEOUT + ) + qtbot.waitUntil( + lambda: gui.typesPane.entryWidgets.get("IF", {}).get("toggle") is not None, + timeout=BROADCAST_TIMEOUT, + ) + gui.typesPane.entryWidgets["IF"]["toggle"].click() + qtbot.waitUntil( + lambda: pm.get_lock("q10.IF") is not None, timeout=BROADCAST_TIMEOUT + ) + qtbot.waitUntil( + lambda: gui.typesPane.entryWidgets.get("IF", {}).get("remove") is not None, + timeout=BROADCAST_TIMEOUT, + ) + + gui.typesPane.entryWidgets["IF"]["remove"].click() + qtbot.waitUntil( + lambda: "IF" not in pm.get_type("qubit").parameters, + timeout=BROADCAST_TIMEOUT, + ) + qtbot.waitUntil( + lambda: not pm.has_param("_globals.qubit.IF"), timeout=BROADCAST_TIMEOUT + ) + assert pm.get_lock("q10.IF") is None + assert pm.has_param("q10.IF") + assert _server_value(pm, "q10.IF") == 1.0 + assert pm.has_param("_globals.qubit.gain") + assert gui.typesPane.entriesNote.text() == "" + finally: + gui.model.stopListener() + + +def test_a_unit_edit_keeps_the_instances_recognised( + qtbot, pm, second_client, server_port +): + """Return in an own entry's unit editor sets the unit on the Type, on + every Instance and on the entry's Globals parameter; the GUI shows the + new unit and still counts the Instance as one of the Type's.""" + second_pm = _second_parameter_manager(second_client) + gui = _make_gui(qtbot, pm, server_port) + try: + _wait_until_broadcasts_arrive(qtbot, gui, second_pm) + _create_type_with_instance(qtbot, gui, pm) + qtbot.waitUntil( + lambda: pm.has_param("_globals.qubit.IF"), timeout=BROADCAST_TIMEOUT + ) + items = _entry_row_items(gui, "IF") + unit_editor = gui.typesPane.entriesView.indexWidget( + gui.typesPane.entriesModel.indexFromItem(items[1]) + ) + assert isinstance(unit_editor, QtWidgets.QLineEdit) + + unit_editor.selectAll() + qtbot.keyClicks(unit_editor, "MHz") + qtbot.keyClick(unit_editor, QtCore.Qt.Key.Key_Return) + qtbot.waitUntil( + lambda: pm.get_type("qubit").parameters["IF"]["unit"] == "MHz", + timeout=BROADCAST_TIMEOUT, + ) + assert _server_unit(pm, "q10.IF") == "MHz" + assert _server_unit(pm, "_globals.qubit.IF") == "MHz" + assert pm.instances_of("qubit") == ["q10"] + + qtbot.waitUntil( + lambda: _row_items(gui, "q10.IF")[1].text() == "MHz", + timeout=BROADCAST_TIMEOUT, + ) + qtbot.waitUntil( + lambda: gui.state.types["qubit"].parameters["IF"]["unit"] == "MHz", + timeout=BROADCAST_TIMEOUT, + ) + assert instances_of_type( + "qubit", gui.state.types, gui.model.parameter_units() + ) == ["q10"] + assert _instance_row_items(gui, "q10") is not None + assert _row_items(gui, "q10.IF")[3].data(GUTTER_ROLE) == ["qubit"] + finally: + gui.model.stopListener() + + +def test_the_instance_delete_button_asks_before_deleting( + qtbot, pm, second_client, server_port, monkeypatch +): + """The delete button asks first; answering No leaves the Instance as + it is.""" + second_pm = _second_parameter_manager(second_client) + gui = _make_gui(qtbot, pm, server_port) + try: + _wait_until_broadcasts_arrive(qtbot, gui, second_pm) + _create_type_with_instance(qtbot, gui, pm) + asked = [] + _answer_dialogs( + monkeypatch, + QtWidgets.QMessageBox.StandardButton.No, + before=lambda: asked.append(True), + ) + + _show, delete = _instance_buttons(gui, "q10") + delete.click() + qtbot.wait(200) + assert asked == [True] + assert pm.has_param("q10.IF") + assert pm.instances_of("qubit") == ["q10"] + assert _instance_row_items(gui, "q10") is not None + finally: + gui.model.stopListener() + + +def test_the_instance_delete_button_removes_the_whole_instance( + qtbot, pm, second_client, server_port, monkeypatch +): + """Answering Yes removes every parameter under the Instance and the + emptied groups; other parameters stay, the instance row goes and the + note stays empty. The proxy is refreshed once for the whole deletion, + not once per parameter.""" + second_pm = _second_parameter_manager(second_client) + gui = _make_gui(qtbot, pm, server_port) + try: + _wait_until_broadcasts_arrive(qtbot, gui, second_pm) + _create_type_with_instance(qtbot, gui, pm) + for path in ("q10.extra.a", "q10.extra.b", "q10.extra.deep.c", "other.x"): + second_pm.add_parameter(path, initial_value=0.0, unit="") + qtbot.waitUntil(lambda: _row_exists(gui, "other.x"), timeout=BROADCAST_TIMEOUT) + qtbot.waitUntil( + lambda: _row_exists(gui, "q10.extra.deep.c"), timeout=BROADCAST_TIMEOUT + ) + _answer_dialogs(monkeypatch, QtWidgets.QMessageBox.StandardButton.Yes) + + updates = [] + original_update = pm.update + + def counting_update(*args, **kwargs): + updates.append(True) + return original_update(*args, **kwargs) + + monkeypatch.setattr(pm, "update", counting_update) + _show, delete = _instance_buttons(gui, "q10") + delete.click() + monkeypatch.setattr(pm, "update", original_update) + + assert not any(path.startswith("q10.") for path in pm.list()) + assert pm.has_param("other.x") + assert pm.instances_of("qubit") == [] + assert len(updates) == 1 + qtbot.waitUntil( + lambda: _instance_row_items(gui, "q10") is None, timeout=BROADCAST_TIMEOUT + ) + qtbot.waitUntil(lambda: not _row_exists(gui, "q10"), timeout=BROADCAST_TIMEOUT) + assert gui.typesPane.instancesNote.text() == "" + finally: + gui.model.stopListener() + + +def test_a_refused_instance_deletion_shows_the_error( + qtbot, pm, second_client, server_port, monkeypatch +): + """When the Server refuses part of the deletion — here a parameter a + second Client removed while the dialog was open — the error shows in + red on the instances pane's note.""" + second_pm = _second_parameter_manager(second_client) + gui = _make_gui(qtbot, pm, server_port) + try: + _wait_until_broadcasts_arrive(qtbot, gui, second_pm) + _create_type_with_instance(qtbot, gui, pm) + _answer_dialogs( + monkeypatch, + QtWidgets.QMessageBox.StandardButton.Yes, + before=lambda: second_pm.remove_parameter("q10.IF"), + ) + + _show, delete = _instance_buttons(gui, "q10") + delete.click() + qtbot.waitUntil( + lambda: gui.typesPane.instancesNote.text() != "", timeout=BROADCAST_TIMEOUT + ) + assert "red" in gui.typesPane.instancesNote.styleSheet() + finally: + gui.model.stopListener() + + +def test_the_gui_creates_the_globals_parameter_of_every_entry( + qtbot, pm, second_client, server_port +): + """Every Type entry gets its Globals parameter + ``_globals..`` with the entry's default and unit — the + entries that existed before the GUI opened, and the ones added through + it — so every entry has a global value to edit. A Globals parameter + never counts as an Instance.""" + second_pm = _second_parameter_manager(second_client) + second_pm.add_type("readout") + second_pm.add_type_parameter("readout", "bw", default=2.0, unit="Hz") + second_pm.add_type_parameter("readout", "sub.gain", default=None, unit="dB") + assert not pm.has_param("_globals.readout.bw") + gui = _make_gui(qtbot, pm, server_port) + try: + _wait_until_broadcasts_arrive(qtbot, gui, second_pm) + qtbot.waitUntil( + lambda: ( + pm.has_param("_globals.readout.bw") + and pm.has_param("_globals.readout.sub.gain") + ), + timeout=BROADCAST_TIMEOUT, + ) + assert _server_value(pm, "_globals.readout.bw") == 2.0 + assert _server_unit(pm, "_globals.readout.bw") == "Hz" + assert _server_value(pm, "_globals.readout.sub.gain") is None + assert _server_unit(pm, "_globals.readout.sub.gain") == "dB" + + _create_type_with_instance(qtbot, gui, pm) + qtbot.waitUntil( + lambda: pm.has_param("_globals.qubit.IF"), timeout=BROADCAST_TIMEOUT + ) + assert _server_value(pm, "_globals.qubit.IF") == 1.0 + assert _server_unit(pm, "_globals.qubit.IF") == "Hz" + assert pm.instances_of("readout") == [] + assert pm.instances_of("qubit") == ["q10"] + qtbot.waitUntil( + lambda: "_globals.qubit.IF" in gui.typesPane.globalEditors, + timeout=BROADCAST_TIMEOUT, + ) + editor, _set_button = _global_widgets(gui.typesPane, "_globals.qubit.IF") + assert editor.text() == "1.0" + finally: + gui.model.stopListener() + + +def test_the_global_editor_sets_the_value_the_locked_instances_follow( + qtbot, pm, second_client, server_port +): + """Setting the global value works locked or not: Return sets the + entry's Globals parameter and turns the button plain again; once the + entry is locked the Instances follow it, and the set button sets it as + well; a value a second Client sets shows in the editor.""" + second_pm = _second_parameter_manager(second_client) + gui = _make_gui(qtbot, pm, server_port) + try: + _wait_until_broadcasts_arrive(qtbot, gui, second_pm) + _create_type_with_instance(qtbot, gui, pm) + target = "_globals.qubit.IF" + qtbot.waitUntil( + lambda: target in gui.typesPane.globalEditors, timeout=BROADCAST_TIMEOUT + ) + + # unlocked: the Globals parameter is set, the Instance keeps its own + editor, set_button = _global_widgets(gui.typesPane, target) + editor.selectAll() + qtbot.keyClicks(editor, "3.5") + assert _is_pending(set_button) + qtbot.keyClick(editor, QtCore.Qt.Key.Key_Return) + qtbot.waitUntil( + lambda: _server_value(pm, target) == 3.5, timeout=BROADCAST_TIMEOUT + ) + qtbot.waitUntil( + lambda: not _is_pending(_global_widgets(gui.typesPane, target)[1]), + timeout=BROADCAST_TIMEOUT, + ) + assert _server_value(pm, "q10.IF") == 1.0 + assert gui.typesPane.entriesNote.text() == "" + + # locked: the Instance follows the global value + gui.typesPane.entryWidgets["IF"]["toggle"].click() + qtbot.waitUntil( + lambda: pm.get_lock("q10.IF") is not None, timeout=BROADCAST_TIMEOUT + ) + assert _server_value(pm, "q10.IF") == 3.5 + qtbot.waitUntil( + lambda: target in gui.typesPane.globalEditors, timeout=BROADCAST_TIMEOUT + ) + editor, set_button = _global_widgets(gui.typesPane, target) + editor.selectAll() + qtbot.keyClicks(editor, "4.5") + set_button.click() + qtbot.waitUntil( + lambda: _server_value(pm, "q10.IF") == 4.5, timeout=BROADCAST_TIMEOUT + ) + qtbot.waitUntil( + lambda: not _is_pending(_global_widgets(gui.typesPane, target)[1]), + timeout=BROADCAST_TIMEOUT, + ) + + # a value set from outside shows in the editor + second_pm.update() + nestedAttributeFromString(second_pm, target)(5.5) + qtbot.waitUntil( + lambda: _global_widgets(gui.typesPane, target)[0].text() == "5.5", + timeout=BROADCAST_TIMEOUT, + ) + finally: + gui.model.stopListener() + + +def test_an_uncommitted_global_edit_is_not_overwritten_from_outside( + qtbot, pm, second_client, server_port +): + """While a global edit is uncommitted, a value a second Client sets + does not replace the typed text; committing then sets the typed + value.""" + second_pm = _second_parameter_manager(second_client) + gui = _make_gui(qtbot, pm, server_port) + try: + _wait_until_broadcasts_arrive(qtbot, gui, second_pm) + _create_type_with_instance(qtbot, gui, pm) + target = "_globals.qubit.IF" + qtbot.waitUntil( + lambda: target in gui.typesPane.globalEditors, timeout=BROADCAST_TIMEOUT + ) + editor, set_button = _global_widgets(gui.typesPane, target) + editor.selectAll() + qtbot.keyClicks(editor, "8.0") + + second_pm.update() + nestedAttributeFromString(second_pm, target)(9.0) + qtbot.wait(500) + editor, set_button = _global_widgets(gui.typesPane, target) + assert editor.text() == "8.0" + assert _is_pending(set_button) + + qtbot.keyClick(editor, QtCore.Qt.Key.Key_Return) + qtbot.waitUntil( + lambda: _server_value(pm, target) == 8.0, timeout=BROADCAST_TIMEOUT + ) + finally: + gui.model.stopListener() + + +def test_a_refused_global_value_shows_the_error(qtbot, pm, second_client, server_port): + """A global value the Server cannot set — a Target that does not + exist — shows the error in red on the entries pane's note.""" + second_pm = _second_parameter_manager(second_client) + gui = _make_gui(qtbot, pm, server_port) + try: + _wait_until_broadcasts_arrive(qtbot, gui, second_pm) + _create_type_with_instance(qtbot, gui, pm) + + gui.typesPane.setGlobalRequested.emit("no_such.parameter", "1.0") + qtbot.waitUntil( + lambda: gui.typesPane.entriesNote.text() != "", timeout=BROADCAST_TIMEOUT + ) + assert "red" in gui.typesPane.entriesNote.styleSheet() + finally: + gui.model.stopListener() + + +def test_the_default_editor_sets_the_entry_default( + qtbot, pm, second_client, server_port +): + """Return and the set button set an own entry's default on the Server, + and the button turns plain once the default is set.""" + second_pm = _second_parameter_manager(second_client) + gui = _make_gui(qtbot, pm, server_port) + try: + _wait_until_broadcasts_arrive(qtbot, gui, second_pm) + _create_type_with_instance(qtbot, gui, pm) + + editor = gui.typesPane.entryWidgets["IF"]["editor"] + editor.selectAll() + qtbot.keyClicks(editor, "2.5") + assert _is_pending(gui.typesPane.entryWidgets["IF"]["set"]) + qtbot.keyClick(editor, QtCore.Qt.Key.Key_Return) + qtbot.waitUntil( + lambda: pm.get_type("qubit").parameters["IF"]["default"] == 2.5, + timeout=BROADCAST_TIMEOUT, + ) + qtbot.waitUntil( + lambda: ( + gui.typesPane.entryWidgets["IF"]["editor"].text() == "2.5" + and not _is_pending(gui.typesPane.entryWidgets["IF"]["set"]) + ), + timeout=BROADCAST_TIMEOUT, + ) + + editor = gui.typesPane.entryWidgets["IF"]["editor"] + editor.selectAll() + qtbot.keyClicks(editor, "3.5") + gui.typesPane.entryWidgets["IF"]["set"].click() + qtbot.waitUntil( + lambda: pm.get_type("qubit").parameters["IF"]["default"] == 3.5, + timeout=BROADCAST_TIMEOUT, + ) + qtbot.waitUntil( + lambda: not _is_pending(gui.typesPane.entryWidgets["IF"]["set"]), + timeout=BROADCAST_TIMEOUT, + ) + # a default only seeds new parameters: the Instance keeps its value + assert _server_value(pm, "q10.IF") == 1.0 + finally: + gui.model.stopListener() + + +def test_the_live_type_lock_has_no_re_target(qtbot, pm, second_client, server_port): + """With the Type Lock re-target gone, a locked entry offers only the + lock toggle, and the GUI has no Type Lock arm to pick a Target for.""" + second_pm = _second_parameter_manager(second_client) + gui = _make_gui(qtbot, pm, server_port) + try: + _wait_until_broadcasts_arrive(qtbot, gui, second_pm) + _create_type_with_instance(qtbot, gui, pm) + gui.typesPane.entryWidgets["IF"]["toggle"].click() + qtbot.waitUntil( + lambda: ( + gui.state.types["qubit"].parameters["IF"]["target"] + == f"{PM_NAME}._globals.qubit.IF" + ), + timeout=BROADCAST_TIMEOUT, + ) + qtbot.waitUntil( + lambda: gui.typesPane.entryWidgets.get("IF", {}).get("toggle") is not None, + timeout=BROADCAST_TIMEOUT, + ) + assert gui.typesPane.entryWidgets["IF"].get("retarget") is None + assert not hasattr(gui.locksController, "arm_type_lock") + assert not hasattr(gui.locksController, "armed_type_lock") + finally: + gui.model.stopListener() + + +def test_a_burst_of_creations_recomputes_the_tints_once( + qtbot, pm, second_client, server_port +): + """A burst of parameter-creation Broadcasts — an Instance created — + emits ``structureChanged`` once, after the burst, instead of once per + parameter; the new rows carry the claiming Type's gutter band.""" + second_pm = _second_parameter_manager(second_client) + second_pm.add_type("qubit") + for name in ("a", "b", "c", "d", "e"): + second_pm.add_type_parameter("qubit", name, default=0.0, unit="Hz") + gui = _make_gui(qtbot, pm, server_port) + try: + _wait_until_broadcasts_arrive(qtbot, gui, second_pm) + gui.model.stopListener() + second_pm.add_instance("qubit", "q20") + pm.update() + emitted = [] + gui.model.structureChanged.connect(lambda: emitted.append(True)) + + for name in ("a", "b", "c", "d", "e"): + gui.model.updateParameter( + ParameterBroadcastBluePrint( + name=f"{PM_NAME}.q20.{name}", + action=PARAMETER_CREATION, + value=0.0, + unit="Hz", + ) + ) + qtbot.waitUntil(lambda: len(emitted) >= 1, timeout=BROADCAST_TIMEOUT) + qtbot.wait(300) + assert len(emitted) == 1 + for name in ("a", "b", "c", "d", "e"): + assert _row_exists(gui, f"q20.{name}") + qtbot.waitUntil( + lambda: _row_items(gui, "q20.a")[3].data(GUTTER_ROLE) == ["qubit"], + timeout=BROADCAST_TIMEOUT, + ) + finally: + gui.model.stopListener() + + +def test_a_burst_of_deletions_recomputes_the_tints_once( + qtbot, pm, second_client, server_port +): + """A burst of parameter-deletion Broadcasts — an Instance removed — + emits ``structureChanged`` once, after the burst.""" + second_pm = _second_parameter_manager(second_client) + for name in ("a", "b", "c", "d", "e"): + second_pm.add_parameter(f"q30.{name}", initial_value=0.0, unit="Hz") + pm.update() + gui = _make_gui(qtbot, pm, server_port) + try: + _wait_until_broadcasts_arrive(qtbot, gui, second_pm) + for name in ("a", "b", "c", "d", "e"): + assert _row_exists(gui, f"q30.{name}") + gui.model.stopListener() + for name in ("a", "b", "c", "d", "e"): + second_pm.remove_parameter(f"q30.{name}") + pm.update() + emitted = [] + gui.model.structureChanged.connect(lambda: emitted.append(True)) + + for name in ("a", "b", "c", "d", "e"): + gui.model.updateParameter( + ParameterBroadcastBluePrint( + name=f"{PM_NAME}.q30.{name}", action=PARAMETER_DELETION + ) + ) + qtbot.waitUntil(lambda: len(emitted) >= 1, timeout=BROADCAST_TIMEOUT) + qtbot.wait(300) + assert len(emitted) == 1 + assert not _row_exists(gui, "q30.a") + finally: + gui.model.stopListener() diff --git a/test/pytest/test_pm_types.py b/test/pytest/test_pm_types.py index 1f286f7..7480d48 100644 --- a/test/pytest/test_pm_types.py +++ b/test/pytest/test_pm_types.py @@ -101,6 +101,19 @@ and the Server's ``parameter-deletion``, the first client's proxy shows the Follower unlocked after ``update()``, and ``get_type`` over the wire shows ``target`` ``None``. +The global values part checks ``ensure_global_targets``: it creates the +Globals parameter of every Type entry that has none, with the entry's +default and unit, under the defining Type only (never under a Type that +merely nests it), returns the created paths, keeps existing Globals +parameters with their values, is idempotent, creates nothing for a Type +without entries, emits one ``parameter-creation`` per created parameter +and no ``pm-type-update``, and leaves the Instance matching alone. It +checks that ``set_type_parameter_unit`` carries the new unit to the +entry's Globals parameter (keeping its value, creating none), so a later +Type Lock declaration on the default Target succeeds. The proxy part +calls ``ensure_global_targets`` over the wire and checks that a proxy's +``update()`` brings the new unit of a parameter it already knew, whether +the unit was changed through the same or a second client. """ import copy @@ -4191,3 +4204,242 @@ def test_removing_a_globals_type_lock_target_over_the_wire( finally: second_cli.disconnect() _cleanup_proxy_deletion_types(params) + + +# --------------------------------------------------------------------------- +# Global values: every Type entry's Globals parameter, its unit following +# the entry, and proxies keeping known units current. +# --------------------------------------------------------------------------- + + +def test_ensure_global_targets_creates_the_missing_globals_parameters(pm): + pm.add_type("qubit") + pm.add_type_parameter("qubit", "IF", default=5e9, unit="Hz") + pm.add_type_parameter("qubit", "pulses.pi.len", default=None, unit="us") + pm.add_type("readout") + pm.add_type_parameter("readout", "bw", default=2.0, unit="Hz") + + created = pm.ensure_global_targets() + + assert sorted(created) == [ + "_globals.qubit.IF", + "_globals.qubit.pulses.pi.len", + "_globals.readout.bw", + ] + # every Globals parameter starts with the entry's default and unit + assert pm.get("_globals.qubit.IF") == 5e9 + assert pm.parameter("_globals.qubit.IF").unit == "Hz" + assert pm.get("_globals.qubit.pulses.pi.len") is None + assert pm.parameter("_globals.qubit.pulses.pi.len").unit == "us" + assert pm.get("_globals.readout.bw") == 2.0 + assert pm.parameter("_globals.readout.bw").unit == "Hz" + # created like the Type Lock's default Target: a ManagedParameter with + # the full dotted path + param = pm.parameter("_globals.qubit.IF") + assert isinstance(param, ManagedParameter) + assert param.path == "parameter_manager._globals.qubit.IF" + + +def test_ensure_global_targets_keeps_existing_globals_and_is_idempotent(pm): + pm.add_type("qubit") + pm.add_type_parameter("qubit", "IF", default=5e9, unit="Hz") + pm.add_type_parameter("qubit", "gain", default=0.5, unit="") + pm._ensure_global_target("qubit", "IF") + pm.set("_globals.qubit.IF", 42.0) + + # only the entry without a Globals parameter gets one; the existing one + # keeps its own value + assert pm.ensure_global_targets() == ["_globals.qubit.gain"] + assert pm.get("_globals.qubit.IF") == 42.0 + assert pm.get("_globals.qubit.gain") == 0.5 + + # a second call has nothing left to create and changes nothing + pm.set("_globals.qubit.gain", 0.75) + before = sorted(pm.list()) + assert pm.ensure_global_targets() == [] + assert sorted(pm.list()) == before + assert pm.get("_globals.qubit.gain") == 0.75 + + +def test_ensure_global_targets_creates_nested_entries_under_their_own_type(pm): + pm.add_type("readout") + pm.add_type_parameter("readout", "IF", default=10e6, unit="Hz") + pm.add_type("qubit") + pm.add_type_parameter("qubit", "octave_gain", default=10, unit="dB") + pm.add_nested_type("qubit", "readout", "readout") + + created = pm.ensure_global_targets() + + # a Nested Type's entries have one global value, the defining Type's: + # qubit only owns octave_gain + assert sorted(created) == ["_globals.qubit.octave_gain", "_globals.readout.IF"] + assert not pm.has_param("_globals.qubit.readout.IF") + + +def test_ensure_global_targets_with_no_entries_creates_nothing(pm): + assert pm.ensure_global_targets() == [] + pm.add_type("empty") + assert pm.ensure_global_targets() == [] + assert not any(path.startswith("_globals") for path in pm.list()) + + +def test_ensure_global_targets_leaves_the_instance_matching_alone(pm): + pm.add_type("qubit") + pm.add_type_parameter("qubit", "IF", default=5e9, unit="Hz") + pm.add_instance("qubit", "q01") + + pm.ensure_global_targets() + + # the Globals submodule carries the Type's shape but is never an + # Instance + assert pm.instances_of("qubit") == ["q01"] + assert pm.types_of("_globals.qubit.IF") == [] + + +def test_ensure_global_targets_emits_one_creation_per_created_parameter( + pm_with_sink, +): + pm, received = pm_with_sink + pm.add_type("qubit") + pm.add_type_parameter("qubit", "IF", default=5e9, unit="Hz") + pm.add_type_parameter("qubit", "gain", default=0.5, unit="") + received.clear() + + pm.ensure_global_targets() + + assert sorted(bp.name for bp in received) == [ + "parameter_manager._globals.qubit.IF", + "parameter_manager._globals.qubit.gain", + ] + assert all(bp.action == PARAMETER_CREATION for bp in received) + by_name = {bp.name: bp for bp in received} + assert by_name["parameter_manager._globals.qubit.IF"].value == 5e9 + assert by_name["parameter_manager._globals.qubit.IF"].unit == "Hz" + # no Type is edited, so no pm-type-update goes out + assert not any(bp.action == PM_TYPE_UPDATE for bp in received) + + # nothing left to create: nothing is emitted + received.clear() + pm.ensure_global_targets() + assert received == [] + + +def test_set_type_parameter_unit_carries_the_unit_to_the_globals_parameter(pm): + pm.add_type("qubit") + pm.add_type_parameter("qubit", "IF", default=5e9, unit="Hz") + pm.add_instance("qubit", "q01") + pm.ensure_global_targets() + pm.set("_globals.qubit.IF", 6e9) + + pm.set_type_parameter_unit("qubit", "IF", "GHz") + + assert pm.parameter("_globals.qubit.IF").unit == "GHz" + # the value stays; only the unit follows the entry + assert pm.get("_globals.qubit.IF") == 6e9 + assert pm.parameter("q01.IF").unit == "GHz" + + +def test_set_type_parameter_unit_creates_no_globals_parameter(pm): + pm.add_type("qubit") + pm.add_type_parameter("qubit", "IF", default=5e9, unit="Hz") + + pm.set_type_parameter_unit("qubit", "IF", "GHz") + + assert not pm.has_param("_globals.qubit.IF") + + +def test_a_type_lock_after_a_unit_edit_uses_the_globals_parameter(pm): + pm.add_type("qubit") + pm.add_type_parameter("qubit", "IF", default=5e9, unit="Hz") + pm.add_instance("qubit", "q01") + pm.ensure_global_targets() + pm.set("_globals.qubit.IF", 6e9) + pm.set_type_parameter_unit("qubit", "IF", "GHz") + + # the Globals parameter carries the entry's unit, so declaring the Type + # Lock on the default Target meets no unit conflict + assert pm.lock_type_parameter("qubit", "IF") == [] + assert pm.get_type("qubit").parameters["IF"]["target"] == ( + "parameter_manager._globals.qubit.IF" + ) + assert pm.get("q01.IF") == 6e9 + + +PROXY_GLOBAL_TYPE = "pglob_qubit" +PROXY_GLOBAL_INSTANCE = "pglob_q01" + + +def _cleanup_proxy_global_types(params): + """Remove every parameter (the Globals ones included) and the Type the + global value proxy tests create, so the module's shared server-side + Parameter Manager starts each test clean.""" + for path in list(params.list()): + parts = path.split(".") + if parts[0].startswith("pglob_") or ( + parts[0] == "_globals" and parts[1].startswith("pglob_") + ): + params.remove_parameter(path) + if PROXY_GLOBAL_TYPE in params.list_types(): + params.remove_type(PROXY_GLOBAL_TYPE) + + +def test_ensure_global_targets_through_the_proxy(param_manager): + cli, params = param_manager + _cleanup_proxy_global_types(params) + try: + params.add_type(PROXY_GLOBAL_TYPE) + params.add_type_parameter(PROXY_GLOBAL_TYPE, "IF", default=5e9, unit="Hz") + + created = params.ensure_global_targets() + + assert f"_globals.{PROXY_GLOBAL_TYPE}.IF" in created + assert params.has_param(f"_globals.{PROXY_GLOBAL_TYPE}.IF") + assert f"_globals.{PROXY_GLOBAL_TYPE}.IF" not in params.ensure_global_targets() + finally: + _cleanup_proxy_global_types(params) + + +def test_a_proxy_update_brings_the_new_unit_of_a_known_parameter(param_manager): + cli, params = param_manager + _cleanup_proxy_global_types(params) + try: + params.add_type(PROXY_GLOBAL_TYPE) + params.add_type_parameter(PROXY_GLOBAL_TYPE, "IF", default=5e9, unit="Hz") + params.add_instance(PROXY_GLOBAL_TYPE, PROXY_GLOBAL_INSTANCE) + params.update() + # the proxy knows the Instance's parameter with its first unit + instance = getattr(params, PROXY_GLOBAL_INSTANCE) + assert instance.IF.unit == "Hz" + + params.set_type_parameter_unit(PROXY_GLOBAL_TYPE, "IF", "GHz") + params.update() + + # update() refreshes the units of the parameters it already had + assert getattr(params, PROXY_GLOBAL_INSTANCE).IF.unit == "GHz" + finally: + _cleanup_proxy_global_types(params) + + +def test_a_proxy_update_brings_a_unit_a_second_client_changed( + param_manager, server_port +): + cli, params = param_manager + _cleanup_proxy_global_types(params) + second_cli = Client(port=server_port) + try: + params.add_type(PROXY_GLOBAL_TYPE) + params.add_type_parameter(PROXY_GLOBAL_TYPE, "IF", default=5e9, unit="Hz") + params.add_instance(PROXY_GLOBAL_TYPE, PROXY_GLOBAL_INSTANCE) + params.update() + assert getattr(params, PROXY_GLOBAL_INSTANCE).IF.unit == "Hz" + + second = second_cli.find_or_create_instrument( + "parameter_manager", "instrumentserver.params.ParameterManager" + ) + second.set_type_parameter_unit(PROXY_GLOBAL_TYPE, "IF", "MHz") + params.update() + + assert getattr(params, PROXY_GLOBAL_INSTANCE).IF.unit == "MHz" + finally: + second_cli.disconnect() + _cleanup_proxy_global_types(params) From c82a55839df65443706d31bec30ba89c1152487a Mon Sep 17 00:00:00 2001 From: Oliver Wolff Date: Wed, 21 Oct 2026 17:47:33 -0500 Subject: [PATCH 10/10] updating to address quality checks --- src/instrumentserver/client/proxy.py | 4 +++- src/instrumentserver/gui/parameter_manager/panels.py | 8 ++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/instrumentserver/client/proxy.py b/src/instrumentserver/client/proxy.py index bc03139..bcad52f 100644 --- a/src/instrumentserver/client/proxy.py +++ b/src/instrumentserver/client/proxy.py @@ -403,7 +403,9 @@ def _getProxyParameters(self) -> None: else: # the Server can change a unit in place (a Type's unit edit # propagates to its Instances), so keep it current - self.parameters[pn].unit = p.unit + param = self.parameters[pn] + if isinstance(param, Parameter): + param.unit = p.unit delKeys = [] for pn in self.parameters.keys(): diff --git a/src/instrumentserver/gui/parameter_manager/panels.py b/src/instrumentserver/gui/parameter_manager/panels.py index 385cd49..35fa5ec 100644 --- a/src/instrumentserver/gui/parameter_manager/panels.py +++ b/src/instrumentserver/gui/parameter_manager/panels.py @@ -1175,9 +1175,13 @@ def _rebuild_entries( ) ) ) - commit = lambda target=target, global_editor=global_editor: ( + + def commit( + target: str = target, + global_editor: QtWidgets.QLineEdit = global_editor, + ) -> None: self.setGlobalRequested.emit(target, global_editor.text()) - ) + global_editor.returnPressed.connect(commit) global_set.pressed.connect(commit) global_layout.addWidget(global_editor, 1)