From 1b6a32d1372c7ac7350aadd2b8f6e6cb2d61f39b Mon Sep 17 00:00:00 2001 From: Dawn Date: Thu, 17 Sep 2026 09:55:09 +0200 Subject: [PATCH 1/9] feat(gui): pin status-bar operables right, stop readout width jitter - Fast Shutter / State / p-group / Session are addPermanentWidget now: QStatusBar itself pins them to the right corner, so wrapping of the other readouts never moves them - passive readouts keep wrapping in the FlowHost on narrow windows - grow-only min-width ratchet on every readout: changing number widths used to re-flow the row each DAQ tick and jitter every neighbour Co-Authored-By: Claude Fable 5 --- src/aare/gui/widgets/status_bar.py | 53 ++++++++++++++++++++++-------- tests/unit/gui/test_status_bar.py | 39 +++++++++++++++------- 2 files changed, 67 insertions(+), 25 deletions(-) diff --git a/src/aare/gui/widgets/status_bar.py b/src/aare/gui/widgets/status_bar.py index 4b31e3f8..a89583d9 100644 --- a/src/aare/gui/widgets/status_bar.py +++ b/src/aare/gui/widgets/status_bar.py @@ -93,16 +93,15 @@ class StatusBar(QStatusBar): self.exp_shutter_label = ValueLabel("ExpHutch Shutter", "", self) - # Every readout lives in ONE wrapping flow host: on narrow windows - # QStatusBar used to clip/hide the labels outright, now they wrap to - # extra rows and stay readable. Fixed order: passives first, then the - # operables (shared ClickableLabel hover affordance) — Cryo | Fast - # Shutter | State | p-group | Session at the end. The old left/right - # split (addWidget vs addPermanentWidget) is gone with the wrap; no - # showMessage is ever used here, so nothing hides the host. - # TODO(cryo): passive for now, but placed with the operables because a - # cryo operation (fill/anneal menu) is planned; when it gets a click - # handler, swap it to a ClickableLabel so it inherits the affordance. + # Split layout: the passive readouts wrap in a flow host (narrow + # windows used to clip them outright), while the four operables — + # Fast Shutter | State | p-group | Session — are addPermanentWidget + # so QStatusBar itself pins them to the right corner at a fixed spot + # regardless of wrapping; no showMessage is ever used here, so + # nothing hides either side. + # TODO(cryo): a cryo operation (fill/anneal menu) is planned; when it + # gets a click handler, swap to ClickableLabel and move it over to + # the permanent (right-pinned) group with the other operables. self.info_host = FlowHost(self) for widget in ( self.message_label, @@ -116,14 +115,22 @@ class StatusBar(QStatusBar): self.tell_state_label, self.busy_label, self.cryo_label, - self.shutter_label, - self.state_label, - self.pgroup_label, - self.session_label, ): self.info_host.add_widget(widget) self.addWidget(self.info_host, 1) + for widget in (self.shutter_label, self.state_label, self.pgroup_label, self.session_label): + self.addPermanentWidget(widget) + + def _ratchet_widths(self, *labels: QLabel) -> None: + # ponytail: grow-only min-width ratchet — per-tick number width + # changes used to re-flow the row and jitter every neighbour; pin + # each label to the widest text it has shown. Never shrinks until + # restart, which is fine for a status bar; per-label fixed widths + # from font metrics if a pathological long value ever sticks. + for label in labels: + label.setMinimumWidth(max(label.minimumWidth(), label.sizeHint().width())) + def set_theme(self, theme: str) -> None: """Adopt the theme's flag colors: recolor the connection message and re-render the DAQ-driven labels from the last status right away.""" @@ -156,6 +163,7 @@ class StatusBar(QStatusBar): @Slot(float) def update_sharpness(self, val: float): self.sharpness.set_value(f"{val:.3f}") + self._ratchet_widths(self.sharpness) @Slot(float) def update_samcam_fps(self, fps: float): @@ -166,6 +174,7 @@ class StatusBar(QStatusBar): self.samcam_fps.set_value("error") return self.samcam_fps.set_value(f"{fps:.1f}") + self._ratchet_widths(self.samcam_fps) @Slot(DAQStatusModel) def update_daq_status(self, status: DAQStatusModel): @@ -284,6 +293,21 @@ class StatusBar(QStatusBar): html_content_session = f"""Session: {session_flag}""" self.session_label.setText(html_content_session) + + self._ratchet_widths( + self.flux, + self.transmission, + self.ring_current, + self.wvl, + self.cryo_label, + self.shutter_label, + self.exp_shutter_label, + self.tell_state_label, + self.busy_label, + self.state_label, + self.pgroup_label, + self.session_label, + ) except Exception: logger.exception("Error updating DAQ status in status bar") @@ -378,6 +402,7 @@ class StatusBar(QStatusBar): text = "Session: Vacant" self.session_label.setText(text) + self._ratchet_widths(self.session_label) def show_session_menu(self, global_pos: QPoint | None = None): menu = QMenu(self) diff --git a/tests/unit/gui/test_status_bar.py b/tests/unit/gui/test_status_bar.py index dc4f4e7f..86312997 100644 --- a/tests/unit/gui/test_status_bar.py +++ b/tests/unit/gui/test_status_bar.py @@ -64,22 +64,39 @@ def test_state_menu_gates_beam_location_for_non_staff(qtbot, daq_status_factory, assert not _state_menu_entries(non_staff)["Beam location (admin mode only)"] -def test_readouts_wrap_instead_of_clipping(qtbot): +def test_readout_width_ratchets_so_numbers_do_not_jitter(qtbot, daq_status_factory): bar = _bar(qtbot) + # A wide value pins the minimum width; a narrower one must NOT shrink it, + # otherwise every tick re-flows the row and neighbours jitter. + bar.update_samcam_fps(1234.5) + bar.update_sharpness(0.123) + wide = bar.samcam_fps.minimumWidth() + assert wide >= bar.samcam_fps.sizeHint().width() + bar.update_samcam_fps(5.0) + assert bar.samcam_fps.minimumWidth() == wide + # The DAQ tick ratchets the rest, session-display path included. + bar.update_daq_status(daq_status_factory()) + bar._update_session_display() + assert bar.ring_current.minimumWidth() >= bar.ring_current.sizeHint().width() + assert bar.session_label.minimumWidth() >= bar.session_label.sizeHint().width() + + +def test_readouts_wrap_instead_of_clipping(qtbot, daq_status_factory): + bar = _bar(qtbot) + # Labels are empty until a DAQ tick paints them; feed one so the flow + # measures realistic widths (empty labels never trigger a wrap). + bar.update_daq_status(daq_status_factory()) host = bar.info_host flow = host.layout() assert isinstance(flow, FlowLayout) # narrows Optional for the checker - # Every readout is in the flow host, none clipped away by QStatusBar. - for label in ( - bar.flux, - bar.busy_label, - bar.cryo_label, - bar.shutter_label, - bar.state_label, - bar.pgroup_label, - bar.session_label, - ): + # Passive readouts wrap in the flow host; the four operables are + # addPermanentWidget so QStatusBar pins them to the right corner and + # they never move when the flow wraps. + for label in (bar.flux, bar.busy_label, bar.cryo_label): assert label.parentWidget() is host + for label in (bar.shutter_label, bar.state_label, bar.pgroup_label, bar.session_label): + assert label.parentWidget() is bar + assert label.parentWidget() is not host # Narrow width -> the flow reports a taller (multi-row) height than one # row, and resizing the host pins its minimum height to the wrapped # height so the bar grows instead of cutting labels off. -- 2.54.0 From 7458ea04006e7151e6e647f5397d1bc23a54c7ed Mon Sep 17 00:00:00 2001 From: Dawn Date: Thu, 17 Sep 2026 10:25:14 +0200 Subject: [PATCH 2/9] chore: better wording, make user calm --- src/aare/daq/server.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/aare/daq/server.py b/src/aare/daq/server.py index e7d32c3d..5525b715 100644 --- a/src/aare/daq/server.py +++ b/src/aare/daq/server.py @@ -94,7 +94,9 @@ async def _lock_hw(): yield else: if not hardware_busy_lock.acquire(blocking=False): - raise BeamlineBusyException("Beamline hardware lock is held by another worker") + raise BeamlineBusyException( + "Beamline is busy with another operation. Please try again later." + ) logger.debug("Hardware lock acquired by process") try: yield -- 2.54.0 From 1ea607ba56cb6ab65113ba6a40f3146f873f5bd4 Mon Sep 17 00:00:00 2001 From: Dawn Date: Thu, 17 Sep 2026 10:41:51 +0200 Subject: [PATCH 3/9] fix(gui): keep queue head until /status confirms unmount on Mount next Mount next popped the mounted head before posting the exchange, so a failed unmount dropped a sample that was still on the gonio. Now the head stays queued and is removed only when /status shows a different (or no) sample mounted; a failed exchange leaves the queue intact for a retry. Co-Authored-By: Claude Fable 5.1 --- src/aare/gui/main_window.py | 52 ++++++++++++++++++++++-------- tests/unit/gui/test_main_window.py | 20 +++++++++++- 2 files changed, 58 insertions(+), 14 deletions(-) diff --git a/src/aare/gui/main_window.py b/src/aare/gui/main_window.py index 21e12fc4..c8d95864 100644 --- a/src/aare/gui/main_window.py +++ b/src/aare/gui/main_window.py @@ -234,6 +234,9 @@ class MainWindow(QMainWindow): self._remote_close_reason: str | None = None self._remote_close_banner_active: bool = False self._latest_daq_status: DAQStatusModel | None = None + # Queue head awaiting removal: popped only once /status shows it is no + # longer on the gonio, so a failed unmount never loses a sample. + self._pending_unmount_pop: int | None = None self._tutorial_event_bus = TutorialEventBus(self) self._tutorial_text_resolver = DictionaryTextResolver(MANUAL_MOUNT_TUTORIAL) @@ -1094,6 +1097,7 @@ class MainWindow(QMainWindow): self.job_list_panel.viewer_track_online.connect(self.viewer.load_online) self.sample_logic.sample_changed.connect(self.data_collection.file_path_panel.update_sample) + self.sample_logic.sample_changed.connect(self._on_mounted_sample_changed) self.daq.update.connect(self.beamline.omega_panel.update_daq_status) self.daq.update.connect(self.beamline.smargon_panel.update_daq_status) @@ -2163,19 +2167,39 @@ class MainWindow(QMainWindow): return None def _mount_next_from_queue(self) -> None: - """Manual step-through: mount queue head; if head already on gonio, pop it and mount the following one. - Pops only after /status confirms mount, so a failed robot move never loses a sample.""" + """Manual step-through: mount queue head; if head already on gonio, mount the following one. + + The mounted head stays in the queue until /status confirms it left the + gonio (see ``_on_mounted_sample_changed``): a failed unmount must not + drop the sample that is still physically mounted. + """ queue = self.job_list_panel.table_model.samples mounted = getattr(self._latest_daq_status, "sample", None) - if queue and mounted is not None and mounted.db_id == queue[0].db_id: - self.job_list_panel.table_model.remove_sample(mounted.db_id) - queue = self.job_list_panel.table_model.samples - if not queue: - self._on_manual_unmount_requested() # or no-op; your call - return - self._on_manual_mount_requested(queue[0]) + head_mounted = bool(queue) and mounted is not None and mounted.db_id == queue[0].db_id + target = queue[1] if head_mounted else (queue[0] if queue else None) + if target is None: + sent = self._on_manual_unmount_requested() + else: + sent = self._on_manual_mount_requested(target) + if sent and head_mounted: + self._pending_unmount_pop = queue[0].db_id - def _on_manual_mount_requested(self, sample, reference: bool = False) -> None: + @Slot(object) + def _on_mounted_sample_changed(self, sample) -> None: + """Pop the pending queue head once /status shows it is off the gonio. + + Intentionally not cleared on ``operation_failed``: a busy/duplicate + POST error must not cancel the pop for an exchange still in flight. + """ + pending = self._pending_unmount_pop + if pending is None: + return + if sample is not None and sample.db_id == pending: + return + self._pending_unmount_pop = None + self.job_list_panel.remove_samples([pending]) + + def _on_manual_mount_requested(self, sample, reference: bool = False) -> bool: """Pre-check the hutch before sending a manual mount to the server. Gives the user an immediate pop-up if the door is open / alarm active, @@ -2188,10 +2212,11 @@ class MainWindow(QMainWindow): QMessageBox.critical(self, "Mounting Failed", reason) except Exception: logger.exception("Failed to show mount-blocked popup") - return + return False self.daq.mount(sample, reference) + return True - def _on_manual_unmount_requested(self) -> None: + def _on_manual_unmount_requested(self) -> bool: """Block a manual unmount if the hutch isn't ready (robot can't move).""" reason = self._hutch_blocks_mount() if reason is not None: @@ -2200,8 +2225,9 @@ class MainWindow(QMainWindow): QMessageBox.critical(self, "Unmounting Failed", reason) except Exception: logger.exception("Failed to show unmount-blocked popup") - return + return False self.daq.unmount() + return True def _precondition_ok(self) -> bool: """Run the shared ring-current / shutter / door 'continue?' check. diff --git a/tests/unit/gui/test_main_window.py b/tests/unit/gui/test_main_window.py index 36b718f3..c7f2e3ab 100644 --- a/tests/unit/gui/test_main_window.py +++ b/tests/unit/gui/test_main_window.py @@ -16,7 +16,7 @@ def mock_ui_state(): yield mock -def test_main_window_init(qtbot, mock_ui_state, daq_status_factory): +def test_main_window_init(qtbot, mock_ui_state, daq_status_factory, sample_info): with ( patch("requests.get") as mock_get, patch("aare.gui.main_window.DAQWorker"), @@ -117,6 +117,24 @@ def test_main_window_init(qtbot, mock_ui_state, daq_status_factory): win._on_manual_unmount_requested() unmount_mock.assert_called_once() + # Mount next: the mounted head stays queued until /status confirms it + # left the gonio, so a failed unmount never drops it from the queue. + head = sample_info + nxt = sample_info.model_copy(update={"db_id": 2, "sample_name": "sample2"}) + win.job_list_panel.table_model.updateData([head, nxt]) + mount_mock = cast(MagicMock, win.daq.mount) + win.update_daq_status(daq_status_factory(sample=head)) + win.sample_logic.update_daq_status(daq_status_factory(sample=head)) + with patch.object(win, "_hutch_blocks_mount", return_value=None): + win._mount_next_from_queue() + mount_mock.assert_called_once_with(nxt, False) + queued = lambda: [s.db_id for s in win.job_list_panel.table_model.samples] + assert queued() == [1, 2], "head must stay queued until unmount confirmed" + win.sample_logic.update_daq_status(daq_status_factory(sample=head)) + assert queued() == [1, 2], "head still mounted (unmount failed) -> keep it" + win.sample_logic.update_daq_status(daq_status_factory(sample=nxt)) + assert queued() == [2], "exchange confirmed by /status -> head popped" + # Motion watch: only the robot station switches to the combined # beamline view. Moving no longer does (users kept losing the sample # camera on short gonio moves), and busy alone never does — Sample -- 2.54.0 From bf11528e8680e5df6a0b853b8a2524dd22f4881a Mon Sep 17 00:00:00 2001 From: Dawn Date: Thu, 17 Sep 2026 11:32:30 +0200 Subject: [PATCH 4/9] fix(gui): zoom-proof the Dewar table and control heights, theme the mounted HUD ink Dewar "#" column sat 1px above the other rows at some zoom steps: the frozen-column overlay sizes its own header from "#" alone while the main header also sees the symbol columns, whose fallback-font glyphs have a taller line box. The overlay header is now pinned to the main header height on resize and (deferred, receiver-bound) on font/style change. Zoom also left columns at their startup widths (headers truncated) and clipped descenders in every button/combo/entry box: the 16px height pin in both sheets was raw px while the body font grew to 18/21px. Columns re-autosize on FontChange and the pin scales with the font ladder. Camera "Currently mounted" HUD: black ink on a white halo in the light themes, white on black in Sunset, instead of white-on-black everywhere. Co-Authored-By: Claude Fable 5.1 --- src/aare/gui/panels/tell_sample_panel.py | 42 ++++++++++++++++++++++-- src/aare/gui/styles.py | 24 +++++++++----- src/aare/gui/widgets/camera_image.py | 11 +++++-- tests/unit/gui/test_camera_image.py | 10 ++++++ tests/unit/gui/test_styles.py | 20 +++++++++++ tests/unit/gui/test_tell_sample_panel.py | 42 ++++++++++++++++++++++++ 6 files changed, 135 insertions(+), 14 deletions(-) create mode 100644 tests/unit/gui/test_styles.py diff --git a/src/aare/gui/panels/tell_sample_panel.py b/src/aare/gui/panels/tell_sample_panel.py index cb20127d..dceb9a0c 100644 --- a/src/aare/gui/panels/tell_sample_panel.py +++ b/src/aare/gui/panels/tell_sample_panel.py @@ -5,7 +5,7 @@ from aarecommon.models.models import ( SampleShortInfo, SampleShortInfoList, ) -from PySide6.QtCore import Qt, Signal, Slot +from PySide6.QtCore import QEvent, Qt, QTimer, Signal, Slot from PySide6.QtWidgets import ( QAbstractItemView, QButtonGroup, @@ -34,6 +34,10 @@ class FrozenColumnTableView(QTableView): FROZEN_WIDTH = 36 + # Emitted after this view's font changed (Ctrl+plus/minus zoom): the + # panel re-autosizes its columns, which were sized once at startup. + font_changed = Signal() + def __init__(self, parent=None): super().__init__(parent) self.frozen = QTableView(self) @@ -71,6 +75,12 @@ class FrozenColumnTableView(QTableView): self._update_frozen_geometry() def _update_frozen_geometry(self) -> None: + # The overlay header must be exactly as tall as the main one, or its + # rows sit above the main rows. Qt sizes a header to its tallest + # visible section, and the main header also sees the symbol columns + # ("⧂", "⌕"), whose fallback-font glyphs have a taller line box than + # "#" at some zoom steps (1px off at 125% on macOS fonts). + self.frozen.horizontalHeader().setFixedHeight(self.horizontalHeader().height()) self.frozen.setGeometry( self.frameWidth(), self.frameWidth(), @@ -82,6 +92,19 @@ class FrozenColumnTableView(QTableView): super().resizeEvent(event) self._update_frozen_geometry() + def changeEvent(self, event) -> None: + super().changeEvent(event) + if event.type() in (QEvent.Type.FontChange, QEvent.Type.StyleChange): + # Zoom/theme re-lay the main header; re-sync the overlay once the + # layout settled. Deferred with `self` as receiver context so a + # pending call dies with the view (no callback into a dead C++ + # object). Not an updateGeometries override or a header-signal + # slot: both get invoked during teardown and a Python callback on + # a half-destroyed view leaves a lost exception behind. + QTimer.singleShot(0, self, self._update_frozen_geometry) + if event.type() == QEvent.Type.FontChange: + self.font_changed.emit() + class QueueDropChip(QPushButton): """Filter chip that doubles as a drop target: dragging table rows onto it @@ -129,6 +152,11 @@ class TellSamplePanel(QFrame): used by the pop-out window so both panels operate on the same data, tints and filters with no syncing.""" super().__init__(parent) + # Before the table view exists: its font_changed slot reads this, and + # a FontChange can already arrive while the view is being parented + # into this panel (raising inside a C++-invoked slot leaves PySide + # with a lost exception and a later "returned NULL" SystemError). + self._columns_autosized = False if samples is None: samples = SampleShortInfoList(s=[]) @@ -215,6 +243,7 @@ class TellSamplePanel(QFrame): self.table_view = FrozenColumnTableView() self.table_view.setShowGrid(False) self.table_view.setAlternatingRowColors(True) + self.table_view.font_changed.connect(self._on_table_font_changed) grid_layout.addWidget(self.table_view, 2, 0, 1, 4) self.table_model = model if model is not None else UserSampleSpreadsheet(samples=samples.s) @@ -263,8 +292,7 @@ class TellSamplePanel(QFrame): # Columns at full content width (horizontal scroll instead of # squishing); done ONCE so later data refreshes don't fight manual - # column adjustments. - self._columns_autosized = False + # column adjustments (zoom re-runs it, see _on_table_font_changed). if self.table_model.rowCount() > 0: self._autosize_columns() @@ -279,6 +307,14 @@ class TellSamplePanel(QFrame): self.table_view.set_frozen_width(FrozenColumnTableView.FROZEN_WIDTH) self._columns_autosized = True + @Slot() + def _on_table_font_changed(self) -> None: + # Zoom changed the glyph widths under columns sized once at startup + # (headers got truncated). Deferred: the view's own font is updated + # only after this signal. Receiver context: cancels if we die first. + if self._columns_autosized: + QTimer.singleShot(0, self, self._autosize_columns) + @Slot(SampleShortInfoList) def new_sample_list(self, samples: SampleShortInfoList): self.table_model.updateData(samples=samples.s) diff --git a/src/aare/gui/styles.py b/src/aare/gui/styles.py index d2a64dbb..8df63336 100644 --- a/src/aare/gui/styles.py +++ b/src/aare/gui/styles.py @@ -613,6 +613,12 @@ FONT_FINE = "11px" # fine print, queue titles FONT_SCALE_LADDER = (1.0, 1.25, 1.5) _font_scale = 1.0 +# Control box height (buttons, combos, entry boxes): scaled with the ladder +# in _palette() like FONT_*. As raw px the 16px cap clipped descenders once +# the 14px body font became 18/21px at the 125/150% stops. +CONTROL_HEIGHT = "16px" +CONTROL_HEIGHT_LOOSE = "24px" # dark-theme button cap (room for icon buttons) + def font_scale() -> float: return _font_scale @@ -680,7 +686,7 @@ def _palette() -> dict[str, str]: # import FONT_* into local f-string QSS keep 1.0 — port them to the # app sheet if zoom must reach them. for k, v in mapping.items(): - if k.startswith("font_") and v.endswith("px"): + if k.startswith(("font_", "control_")) and v.endswith("px"): mapping[k] = f"{round(int(v[:-2]) * _font_scale)}px" return mapping @@ -780,8 +786,8 @@ def _sunrise_stylesheet(overrides: dict[str, str] | None = None) -> str: QPushButton, QToolButton, QComboBox { background-color: $button_bg; border: 1px solid $button_border; - min-height: 16px; - max-height: 16px; + min-height: $control_height; + max-height: $control_height; padding: 1px 8px; } @@ -799,8 +805,8 @@ def _sunrise_stylesheet(overrides: dict[str, str] | None = None) -> str: QLineEdit, QAbstractSpinBox { background-color: $input_bg; border: 1px solid $button_border; - min-height: 16px; - max-height: 16px; + min-height: $control_height; + max-height: $control_height; padding: 1px 6px; } @@ -1320,8 +1326,8 @@ def _sunset_stylesheet() -> str: QPushButton, QToolButton, QComboBox { background-color: $dark_elevated; border: 1px solid $dark_border_faint; - min-height: 16px; - max-height: 24px; + min-height: $control_height; + max-height: $control_height_loose; padding-top: 1px; padding-bottom: 1px; } @@ -1332,8 +1338,8 @@ def _sunset_stylesheet() -> str: QLineEdit, QAbstractSpinBox { background-color: $dark_input_bg; border: 1px solid $dark_border_faint; - min-height: 16px; - max-height: 16px; + min-height: $control_height; + max-height: $control_height; padding-top: 1px; padding-bottom: 1px; } diff --git a/src/aare/gui/widgets/camera_image.py b/src/aare/gui/widgets/camera_image.py index 04705f8b..dc3b42b2 100644 --- a/src/aare/gui/widgets/camera_image.py +++ b/src/aare/gui/widgets/camera_image.py @@ -1277,6 +1277,12 @@ class SampleCameraImageLabel(QGraphicsView): label = f"{bar_um / 1000.0:g} mm" if bar_um >= 1000.0 else f"{bar_um:g} µm" return bar_um, label + def _mounted_hud_ink(self) -> tuple[str, str]: + """(text, shadow) for the mounted-sample HUD: black on a white halo in + the light themes, white on black in Sunset. The camera image behind + is arbitrary, so the pair follows the theme rather than the pixels.""" + return (WHITE, SHADOW) if self._dark_theme else (SHADOW, WHITE) + def _draw_mounted_sample(self, painter: QPainter): # Top-left HUD line (legend sits bottom-left, scale bar bottom-right): # which sample is on the gonio, readable without leaving the camera. @@ -1297,9 +1303,10 @@ class SampleCameraImageLabel(QGraphicsView): margin = 18 text = f"Currently mounted: {self._mounted_sample_name}" baseline = margin + fm.ascent() - painter.setPen(QPen(qcolor(SHADOW, 200))) + ink, halo = self._mounted_hud_ink() + painter.setPen(QPen(qcolor(halo, 200))) painter.drawText(QPointF(margin + 1, baseline + 1), text) - painter.setPen(QPen(qcolor(WHITE))) + painter.setPen(QPen(qcolor(ink))) painter.drawText(QPointF(margin, baseline), text) painter.restore() diff --git a/tests/unit/gui/test_camera_image.py b/tests/unit/gui/test_camera_image.py index bf28c75e..51ff331a 100644 --- a/tests/unit/gui/test_camera_image.py +++ b/tests/unit/gui/test_camera_image.py @@ -324,3 +324,13 @@ def test_mounted_sample_hud_follows_status(camera): # Unmount (sample gone from the status) clears the line again. camera.update_daq_status(status) assert camera._mounted_sample_name is None + + +def test_mounted_hud_ink_follows_theme(camera): + """Mounted-sample HUD: dark ink + white halo on light themes, inverted on Sunset.""" + from aare.gui.styles import SHADOW, THEME_SUNRISE, THEME_SUNSET, WHITE + + camera.set_theme(THEME_SUNRISE) + assert camera._mounted_hud_ink() == (SHADOW, WHITE) + camera.set_theme(THEME_SUNSET) + assert camera._mounted_hud_ink() == (WHITE, SHADOW) diff --git a/tests/unit/gui/test_styles.py b/tests/unit/gui/test_styles.py new file mode 100644 index 00000000..8916e62e --- /dev/null +++ b/tests/unit/gui/test_styles.py @@ -0,0 +1,20 @@ +"""App stylesheet knobs that must track the font zoom ladder.""" + +from aare.gui import styles + + +def test_control_height_scales_with_font_ladder(): + """Buttons/inputs pinned at 16px clipped descenders at 125/150%; the pin + must scale with the ladder in both sheets.""" + + try: + styles.set_font_scale(1.0) + for theme in (styles.THEME_SUNRISE, styles.THEME_SUNSET): + assert "max-height: 16px" in styles.build_app_stylesheet(theme), theme + styles.set_font_scale(1.5) + for theme in (styles.THEME_SUNRISE, styles.THEME_SUNSET): + sheet = styles.build_app_stylesheet(theme) + assert "max-height: 24px" in sheet, theme + assert "max-height: 16px" not in sheet, theme + finally: + styles.set_font_scale(1.0) diff --git a/tests/unit/gui/test_tell_sample_panel.py b/tests/unit/gui/test_tell_sample_panel.py index 2774afb0..0ccd940a 100644 --- a/tests/unit/gui/test_tell_sample_panel.py +++ b/tests/unit/gui/test_tell_sample_panel.py @@ -142,3 +142,45 @@ def test_selected_samples_follow_the_click(panel): assert [s.db_id for s in panel._selected_samples(0)] == [row_ids[0]] # Click outside the selection: only the clicked row is acted on. assert [s.db_id for s in panel._selected_samples(2)] == [row_ids[2]] + + +def test_font_zoom_keeps_frozen_column_aligned_and_reautosizes(panel, qtbot): + """Ctrl+plus zoom: the frozen "#" overlay header must stay exactly as + tall as the main header (else its rows sit 1px higher), and the columns + sized once at startup must re-autosize for the wider glyphs.""" + from PySide6.QtGui import QFont + from PySide6.QtWidgets import QApplication + + from aare.gui import styles + + app = QApplication.instance() + assert isinstance(app, QApplication) # font() lives on QApplication, not QCoreApplication + base = QFont(app.font()) + # Startup look first: under a styled ancestor (MainWindow) fonts reach + # the view through the QSS path; without any sheet Qt would not + # propagate the app font to a child that carries WA_StyleSheet. + panel.setStyleSheet(styles.build_app_stylesheet(styles.THEME_SUNRISE)) + panel.resize(600, 300) + panel.show() + qtbot.waitExposed(panel) + view = panel.table_view + width_before = view.columnWidth(1) + + def row0_top(v): + return v.viewport().mapTo(panel, v.viewport().rect().topLeft()).y() + v.rowViewportPosition( + 0 + ) + + try: + # Same three steps as MainWindow._apply_theme on a zoom change. + styles.set_font_scale(1.25) + big = QFont(base) + big.setPointSizeF(base.pointSizeF() * 1.25) + app.setFont(big) + panel.setStyleSheet(styles.build_app_stylesheet(styles.THEME_SUNRISE)) + qtbot.waitUntil(lambda: view.columnWidth(1) > width_before) + assert view.frozen.horizontalHeader().height() == view.horizontalHeader().height() + assert row0_top(view.frozen) == row0_top(view) + finally: + styles.set_font_scale(1.0) + app.setFont(base) -- 2.54.0 From 8798a815555dd41f76a455cf534922bbbb940f69 Mon Sep 17 00:00:00 2001 From: Dawn Date: Thu, 17 Sep 2026 12:09:39 +0200 Subject: [PATCH 5/9] fix(gui): pop up "File already exists" instead of silently bumping the run The Run guard called set_scan_kind(), which runs update_filename() and skips to the next free run number before the existence check, so the check never fired and the scan started under a new number without a word. Now the check runs first, then a warning box names the existing master file and the run number it moved to, and the run is blocked. - Simple tab "Run rotation" had no guard at all; same guard added - "Taken" also matches _*_master.h5: X-ray Centering writes _raster2d_master.h5, which the exact name missed - Camera context-menu "Evaluate grid" presses the panel button, so it gets the guard and stays inert while the button is disabled - Dead next_free_run_from() removed Co-Authored-By: Claude Fable 5.1 --- src/aare/gui/main_window.py | 4 +- src/aare/gui/panels/file_path_panel.py | 62 ++++++-------- src/aare/gui/panels/smart_rotation_panel.py | 7 ++ .../unit/gui/test_data_collection_settings.py | 84 +++++++++++++++++++ 4 files changed, 120 insertions(+), 37 deletions(-) diff --git a/src/aare/gui/main_window.py b/src/aare/gui/main_window.py index c8d95864..87f0621d 100644 --- a/src/aare/gui/main_window.py +++ b/src/aare/gui/main_window.py @@ -1007,7 +1007,9 @@ class MainWindow(QMainWindow): self.sample_camera.autofocus.connect(self.daq.autofocus) - self.sample_camera.evaluate_grid.connect(self.raster.run_grid_scan) + # Context-menu "Evaluate grid" presses the panel button so it gets the + # same file-exists guard, and stays inert while the button is disabled. + self.sample_camera.evaluate_grid.connect(self.data_collection.raster.start_button.click) self.data_collection.raster.evaluate_grid.connect(self.raster.run_grid_scan) self.data_collection.raster.evaluate_grid_auto.connect(self.raster.run_grid_scan_auto) diff --git a/src/aare/gui/panels/file_path_panel.py b/src/aare/gui/panels/file_path_panel.py index b18521fa..7af162b3 100644 --- a/src/aare/gui/panels/file_path_panel.py +++ b/src/aare/gui/panels/file_path_panel.py @@ -1,3 +1,4 @@ +import glob import os from datetime import datetime from pathlib import Path @@ -119,8 +120,13 @@ class FilePathPanel(QWidget): def _exists_for_run(self, expanded_base_with_run: str) -> bool: # expanded_base_with_run is the base without scan-kind transforms yet effective = self._effective_dataset_base(expanded_base_with_run) - # Consider master file and directory as taken - return os.path.exists(f"{effective}_master.h5") or os.path.exists(effective) + # Taken = master file, directory, or any derived dataset of this run: + # X-ray centering writes "_raster2d_master.h5", not "_master.h5". + return ( + os.path.exists(f"{effective}_master.h5") + or os.path.exists(effective) + or bool(glob.glob(f"{effective}_*_master.h5")) + ) def update_filename(self): dir_name = self.directory_edit.text() @@ -158,7 +164,7 @@ class FilePathPanel(QWidget): # Preview label shows the effective path (what will be written) effective = self._effective_dataset_base(self._filename) - exists = os.path.exists(f"{effective}_master.h5") or os.path.exists(effective) + exists = self._exists_for_run(self._filename) self.file_name_label.setText(effective + "_master.h5") # Empty stylesheet = reset to the THEME text color (a hardcoded # "default" black would be invisible on the dark theme). @@ -231,42 +237,26 @@ class FilePathPanel(QWidget): def effective_path_for_base(self, base_with_run: str) -> str: return self._effective_dataset_base(base_with_run) - def next_free_run_from(self, start_rn: int) -> tuple[int, str]: - # Compute next free run number and updated base - dir_name = self.directory_edit.text().replace("{prefix}", self.file_prefix_edit.text()) - base = dir_name if dir_name.endswith("/") or dir_name == "" else dir_name + "/" - base += "run" if self.file_prefix_edit.text() == "" else self.file_prefix_edit.text() + def file_path_error_box(self, scan_kind: str) -> bool: + """Click-time guard for the Run buttons: True when the run may start. - rn = start_rn - while rn <= self.run_number_edit.maximum(): - candidate_base = self._expand_macros(base, rn) - if not self._exists_for_run(candidate_base): - return rn, candidate_base - rn += 1 - return start_rn, self._expand_macros(base, start_rn) - - def file_path_error_box(self, scan_kind: str): - self.set_scan_kind(scan_kind) - base = self.filename # same base we emit - effective = self.effective_path_for_base(base) - if os.path.exists(f"{effective}_master.h5") or os.path.exists(effective): - reply = QMessageBox.question( + The scan kind is set directly, not via set_scan_kind(): that calls + update_filename(), which silently skips to the next free run number, + so the existence check below could never fire and the user was never + told the file was already there. + """ + self._scan_kind = scan_kind + exists = self._exists_for_run(self.filename) + path = self.effective_path_for_base(self.filename) + "_master.h5" + # Refresh the label for the clicked kind; on a clash this also moves + # the run number to the next free one, as every edit already does. + self.update_filename() + if exists: + QMessageBox.warning( self, "File exists", - # f"This file already exists:\n{effective}_master.h5\nDo you wish to overwrite?", - f"This file already exists:\n{effective}_master.h5\n Updating run number", - QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No, - QMessageBox.StandardButton.No, + f"File already exists:\n{path}\n\n" + f"Run number moved to {self.run_number_edit.value()}.", ) - if reply == QMessageBox.StandardButton.No: - # bump run and update - curr = self.run_number_edit.value() - new_rn, _ = self.next_free_run_from(curr + 1) - self.run_number_edit.setValue(new_rn) - self.update_filename() - return False - else: - return False return False - return True diff --git a/src/aare/gui/panels/smart_rotation_panel.py b/src/aare/gui/panels/smart_rotation_panel.py index 2b1e6d95..e86a8031 100644 --- a/src/aare/gui/panels/smart_rotation_panel.py +++ b/src/aare/gui/panels/smart_rotation_panel.py @@ -515,6 +515,13 @@ class SimpleRotationSettingsPanel(QWidget): @Slot() def run_measurement(self): + # Same file-exists guard as the Rotation tab; this panel is not a + # ScanSettingsPanel, so it has no check_before_run() to inherit. + file_path_panel = getattr(self.parent(), "file_path_panel", None) + if file_path_panel is not None and not file_path_panel.file_path_error_box( + scan_kind="rotation" + ): + return # Send exactly what the panel last calculated and displayed, rather # than re-reading the widgets: dtz, exposure and transmission are a # single consistent solution and must not be mixed with a newer entry. diff --git a/tests/unit/gui/test_data_collection_settings.py b/tests/unit/gui/test_data_collection_settings.py index 1d2ca541..72f50ddc 100644 --- a/tests/unit/gui/test_data_collection_settings.py +++ b/tests/unit/gui/test_data_collection_settings.py @@ -485,3 +485,87 @@ def test_transmission_rows_are_per_mode(panel, qapp, diffraction): raster_label = _grid_widget(raster._layout, 2, 0) assert isinstance(raster_label, QLabel) assert raster_label.text() == "Transmission" + + +# --------------------------------------------------------------------------- +# File path panel: the Run buttons must not overwrite an existing dataset +# --------------------------------------------------------------------------- + + +@pytest.fixture +def file_panel(qapp): + from aare.gui.panels.file_path_panel import FilePathPanel + + p = FilePathPanel() + p.directory_edit.setText("d") + p.file_prefix_edit.setText("x") + p.run_number_edit.setValue(1) + return p + + +def _taken(monkeypatch, *suffixes): + from aare.gui.panels import file_path_panel + + monkeypatch.setattr( + file_path_panel.os.path, "exists", lambda path: any(path.endswith(s) for s in suffixes) + ) + + +def test_run_is_blocked_with_a_popup_when_the_file_exists(file_panel, monkeypatch): + from PySide6.QtWidgets import QMessageBox + + _taken(monkeypatch, "data/d/x_001_master.h5") + boxes = [] + monkeypatch.setattr(QMessageBox, "warning", lambda *a, **k: boxes.append(a)) + + assert file_panel.file_path_error_box(scan_kind="rotation") is False + assert len(boxes) == 1 + assert "data/d/x_001_master.h5" in boxes[0][2] + # The panel moved on to the next free run, so the next click can go ahead. + assert file_panel.run_number_edit.value() == 2 + assert file_panel.file_path_error_box(scan_kind="rotation") is True + assert len(boxes) == 1 + + +def test_run_proceeds_when_the_file_is_free(file_panel, monkeypatch): + from PySide6.QtWidgets import QMessageBox + + _taken(monkeypatch) + monkeypatch.setattr(QMessageBox, "warning", lambda *a, **k: pytest.fail("no popup expected")) + + assert file_panel.file_path_error_box(scan_kind="rotation") is True + assert file_panel.run_number_edit.value() == 1 + + +def test_a_derived_dataset_counts_as_taken(file_panel, monkeypatch): + # X-ray centering writes _raster2d_master.h5, not _master.h5. + from aare.gui.panels import file_path_panel + + _taken(monkeypatch) + monkeypatch.setattr( + file_path_panel.glob, + "glob", + lambda pat: ["hit"] if pat.endswith("raster/d/x_001_*_master.h5") else [], + ) + monkeypatch.setattr(file_path_panel.QMessageBox, "warning", lambda *a, **k: None) + + assert file_panel.file_path_error_box(scan_kind="raster") is False + assert file_panel.run_number_edit.value() == 2 + + +def test_simple_tab_run_is_blocked_by_the_file_guard(qapp): + from PySide6.QtWidgets import QWidget + + from aare.gui.panels.smart_rotation_panel import SimpleRotationSettingsPanel + + holder = QWidget() + cast(Any, holder).file_path_panel = types.SimpleNamespace( + file_path_error_box=lambda scan_kind: False + ) + panel = SimpleRotationSettingsPanel(parent=holder) + requests = [] + panel.rotation_scan.connect(requests.append) + + panel.run_measurement() + + assert requests == [] -- 2.54.0 From 646de672917ce8ed6a05803f157290bbbf9c60a7 Mon Sep 17 00:00:00 2001 From: Dawn Date: Thu, 17 Sep 2026 12:34:23 +0200 Subject: [PATCH 6/9] fix(gui): make the file-exists guard literal: popup, run +1, no scan update_filename() still skipped to the next free run number on every edit and refresh, so the name was always free by the time Run was clicked and the guard never fired. The name is now exactly what the fields say (label turns red on a clash); on Run, an existing target pops "File already exists", bumps the run number by one and does nothing else. Co-Authored-By: Claude Fable 5.1 --- src/aare/gui/panels/file_path_panel.py | 51 +++++++------------ .../unit/gui/test_data_collection_settings.py | 8 ++- 2 files changed, 25 insertions(+), 34 deletions(-) diff --git a/src/aare/gui/panels/file_path_panel.py b/src/aare/gui/panels/file_path_panel.py index 7af162b3..a8ed8a28 100644 --- a/src/aare/gui/panels/file_path_panel.py +++ b/src/aare/gui/panels/file_path_panel.py @@ -144,23 +144,11 @@ class FilePathPanel(QWidget): base += "run" if file_prefix == "" else file_prefix - # Find next free run number using effective dataset path - rn = run_number - while True: - candidate_base = self._expand_macros(base, rn) - if not self._exists_for_run(candidate_base): - break - rn += 1 - if rn > self.run_number_edit.maximum(): - break - - if rn != run_number: - self.run_number_edit.blockSignals(True) - self.run_number_edit.setValue(rn) - self.run_number_edit.blockSignals(False) - + # Literal: the name is exactly what the fields say. No silent skip to + # the next free run here, or the click-time guard below never sees a + # clash; the label just turns red until the user (or the guard) acts. # Store the GUI’s base (without applying scan-kind transforms) for wiring into requests later - self._filename = self._expand_macros(base, rn) + self._filename = self._expand_macros(base, run_number) # Preview label shows the effective path (what will be written) effective = self._effective_dataset_base(self._filename) @@ -240,23 +228,20 @@ class FilePathPanel(QWidget): def file_path_error_box(self, scan_kind: str) -> bool: """Click-time guard for the Run buttons: True when the run may start. - The scan kind is set directly, not via set_scan_kind(): that calls - update_filename(), which silently skips to the next free run number, - so the existence check below could never fire and the user was never - told the file was already there. + On a clash: bump the run number by one, tell the user, refuse the run. + The next click checks the new name again, so nothing is ever skipped + without the user seeing it. """ self._scan_kind = scan_kind - exists = self._exists_for_run(self.filename) + self.update_filename() # label/path for the clicked kind (screening vs rotation) + if not self._exists_for_run(self.filename): + return True path = self.effective_path_for_base(self.filename) + "_master.h5" - # Refresh the label for the clicked kind; on a clash this also moves - # the run number to the next free one, as every edit already does. - self.update_filename() - if exists: - QMessageBox.warning( - self, - "File exists", - f"File already exists:\n{path}\n\n" - f"Run number moved to {self.run_number_edit.value()}.", - ) - return False - return True + self.increment_run_number() + QMessageBox.warning( + self, + "File exists", + f"File already exists:\n{path}\n\n" + f"Run number increased to {self.run_number_edit.value()}.", + ) + return False diff --git a/tests/unit/gui/test_data_collection_settings.py b/tests/unit/gui/test_data_collection_settings.py index 72f50ddc..a6c54c92 100644 --- a/tests/unit/gui/test_data_collection_settings.py +++ b/tests/unit/gui/test_data_collection_settings.py @@ -518,10 +518,16 @@ def test_run_is_blocked_with_a_popup_when_the_file_exists(file_panel, monkeypatc boxes = [] monkeypatch.setattr(QMessageBox, "warning", lambda *a, **k: boxes.append(a)) + # Editing never skips a taken run silently: the name stays literal and + # the label only turns red. + file_panel.set_scan_kind("rotation") + assert file_panel.run_number_edit.value() == 1 + assert file_panel.file_name_label.styleSheet() != "" + assert file_panel.file_path_error_box(scan_kind="rotation") is False assert len(boxes) == 1 assert "data/d/x_001_master.h5" in boxes[0][2] - # The panel moved on to the next free run, so the next click can go ahead. + # Run number went up by one, so the next click can go ahead. assert file_panel.run_number_edit.value() == 2 assert file_panel.file_path_error_box(scan_kind="rotation") is True assert len(boxes) == 1 -- 2.54.0 From 010513e1412006c6cd8e808386574db101463300 Mon Sep 17 00:00:00 2001 From: Dawn Date: Thu, 17 Sep 2026 12:43:08 +0200 Subject: [PATCH 7/9] fix(gui): reach the file-exists guard from every tab, test exact names The tabs are handed parent=DataCollectionSettings but QStackedWidget.addWidget() reparents them to the stack, so the guard's parent() lookup never found file_path_panel and every Run (rotation, screening, raster, X-ray centering, Simple) skipped the check. Lookup now walks up the widget tree; regression test drives all five runs through the real DataCollectionSettings. The "taken" test is exact again: _master.h5 plus the DAQ's derived _raster2d/_raster1d_master.h5. The directory test and the _*_master.h5 glob are gone: neither is a file a run writes, and they painted run numbers red that nothing would ever produce. Co-Authored-By: Claude Fable 5.1 --- src/aare/gui/panels/file_path_panel.py | 31 +++++++++---- src/aare/gui/panels/scan_settings_panel.py | 7 +-- src/aare/gui/panels/smart_rotation_panel.py | 3 +- .../unit/gui/test_data_collection_settings.py | 46 ++++++++++++++++--- 4 files changed, 69 insertions(+), 18 deletions(-) diff --git a/src/aare/gui/panels/file_path_panel.py b/src/aare/gui/panels/file_path_panel.py index a8ed8a28..f897c9b7 100644 --- a/src/aare/gui/panels/file_path_panel.py +++ b/src/aare/gui/panels/file_path_panel.py @@ -1,4 +1,3 @@ -import glob import os from datetime import datetime from pathlib import Path @@ -19,6 +18,22 @@ from aare.gui.widgets.title_label import TitleLabel ## 5. If sample is registered in the database as manual, it is by default placed in /manual/ +def find_file_path_panel(widget) -> "FilePathPanel | None": + """The FilePathPanel that owns ``widget``'s tab, or None outside the GUI. + + Walks up the widget tree: the scan panels are handed parent=DataCollectionSettings + but QStackedWidget.addWidget() reparents them to the stack, so a plain + parent() lookup finds no file_path_panel and the Run guard is skipped. + """ + w = widget.parent() + while w is not None: + panel = getattr(w, "file_path_panel", None) + if panel is not None: + return panel + w = w.parent() + return None + + class FilePathPanel(QWidget): path_updated = Signal(str) @@ -117,16 +132,16 @@ class FilePathPanel(QWidget): pass return str(root / p) + # Master files a run can produce: plain scans write "_master.h5", + # X-ray Centering writes "_raster2d_master.h5" / "_raster1d_master.h5" + # (see daq/operations/raster/service.py). Exact names only: a glob or a + # directory test marks runs red that nothing will ever write. + _MASTER_SUFFIXES = ("_master.h5", "_raster2d_master.h5", "_raster1d_master.h5") + def _exists_for_run(self, expanded_base_with_run: str) -> bool: # expanded_base_with_run is the base without scan-kind transforms yet effective = self._effective_dataset_base(expanded_base_with_run) - # Taken = master file, directory, or any derived dataset of this run: - # X-ray centering writes "_raster2d_master.h5", not "_master.h5". - return ( - os.path.exists(f"{effective}_master.h5") - or os.path.exists(effective) - or bool(glob.glob(f"{effective}_*_master.h5")) - ) + return any(os.path.exists(effective + suffix) for suffix in self._MASTER_SUFFIXES) def update_filename(self): dir_name = self.directory_edit.text() diff --git a/src/aare/gui/panels/scan_settings_panel.py b/src/aare/gui/panels/scan_settings_panel.py index e732f3dd..59faed7e 100644 --- a/src/aare/gui/panels/scan_settings_panel.py +++ b/src/aare/gui/panels/scan_settings_panel.py @@ -42,6 +42,7 @@ from PySide6.QtWidgets import ( ) from aare.gui.constants import LOGGER_NAME +from aare.gui.panels.file_path_panel import find_file_path_panel from aare.gui.widgets.message_box import precondition_check from aare.gui.widgets.number_line_edit import NumberLineEdit @@ -388,9 +389,9 @@ class ScanSettingsPanel(QWidget): ): logger.warning("Beamline not ready; user chose not to continue scan") return False - p = self.parent() - if hasattr(p, "file_path_panel"): - reply = p.file_path_panel.file_path_error_box(scan_kind=scan_kind) + file_path_panel = find_file_path_panel(self) + if file_path_panel is not None: + reply = file_path_panel.file_path_error_box(scan_kind=scan_kind) logger.debug(f"reply from file path panel: {reply}") if not reply: logger.warning("Error with file path.") diff --git a/src/aare/gui/panels/smart_rotation_panel.py b/src/aare/gui/panels/smart_rotation_panel.py index e86a8031..fb6e0b5c 100644 --- a/src/aare/gui/panels/smart_rotation_panel.py +++ b/src/aare/gui/panels/smart_rotation_panel.py @@ -9,6 +9,7 @@ from PySide6.QtCore import Qt, Signal, Slot from PySide6.QtWidgets import QGridLayout, QLabel, QPushButton, QSizePolicy, QSpacerItem, QWidget from aare.gui.constants import LOGGER_NAME +from aare.gui.panels.file_path_panel import find_file_path_panel from aare.gui.panels.rotation_data_collection import ( MAX_OMEGA_SPEED_DEG_S, MIN_EXP_TIME_S, @@ -517,7 +518,7 @@ class SimpleRotationSettingsPanel(QWidget): def run_measurement(self): # Same file-exists guard as the Rotation tab; this panel is not a # ScanSettingsPanel, so it has no check_before_run() to inherit. - file_path_panel = getattr(self.parent(), "file_path_panel", None) + file_path_panel = find_file_path_panel(self) if file_path_panel is not None and not file_path_panel.file_path_error_box( scan_kind="rotation" ): diff --git a/tests/unit/gui/test_data_collection_settings.py b/tests/unit/gui/test_data_collection_settings.py index a6c54c92..1308b9a9 100644 --- a/tests/unit/gui/test_data_collection_settings.py +++ b/tests/unit/gui/test_data_collection_settings.py @@ -547,18 +547,52 @@ def test_a_derived_dataset_counts_as_taken(file_panel, monkeypatch): # X-ray centering writes _raster2d_master.h5, not _master.h5. from aare.gui.panels import file_path_panel - _taken(monkeypatch) - monkeypatch.setattr( - file_path_panel.glob, - "glob", - lambda pat: ["hit"] if pat.endswith("raster/d/x_001_*_master.h5") else [], - ) + _taken(monkeypatch, "raster/d/x_001_raster2d_master.h5") monkeypatch.setattr(file_path_panel.QMessageBox, "warning", lambda *a, **k: None) assert file_panel.file_path_error_box(scan_kind="raster") is False assert file_panel.run_number_edit.value() == 2 +def test_a_directory_or_a_sibling_run_is_not_taken(file_panel, monkeypatch): + # Only the exact master files count; anything else would paint runs red + # that nothing will write. + _taken(monkeypatch, "raster/d/x_001", "raster/d/x_0011_master.h5", "raster/d/x_001_data.h5") + file_panel.set_scan_kind("raster") + assert file_panel.file_name_label.styleSheet() == "" + assert file_panel.file_path_error_box(scan_kind="raster") is True + + +def test_every_tab_reaches_the_file_guard(settings_panel, monkeypatch): + # The tabs live in a QStackedWidget, which reparents them: a parent() + # lookup of file_path_panel found nothing and every Run skipped the guard. + from aare.gui.panels import scan_settings_panel + + asked = [] + monkeypatch.setattr( + settings_panel.file_path_panel, + "file_path_error_box", + lambda scan_kind: asked.append(scan_kind) or False, + ) + monkeypatch.setattr(scan_settings_panel, "precondition_check", lambda *a, **k: True) + scans = [] + for panel in (settings_panel.screening, settings_panel.raster): + panel._beamline_state = BeamlineStateEnum.SampleAlignment + settings_panel.screening.rotation_scan.connect(scans.append) + settings_panel.simple.rotation_scan.connect(scans.append) + settings_panel.raster.evaluate_grid.connect(lambda: scans.append("grid")) + settings_panel.raster.evaluate_grid_auto.connect(lambda: scans.append("auto")) + + settings_panel.screening.run_screening() + settings_panel.screening.run_measurement() + settings_panel.simple.run_measurement() + settings_panel.raster._on_evaluate_clicked() + settings_panel.raster._on_evaluate_auto_clicked() + + assert asked == ["screening", "rotation", "rotation", "raster", "raster"] + assert scans == [] + + def test_simple_tab_run_is_blocked_by_the_file_guard(qapp): from PySide6.QtWidgets import QWidget -- 2.54.0 From d70cac45bf910d644601102170345ba130d47c89 Mon Sep 17 00:00:00 2001 From: Dawn Date: Thu, 17 Sep 2026 12:52:11 +0200 Subject: [PATCH 8/9] fix(gui): keep the file-name preview in the plain theme color Dawn: the red "file exists" tint on the path label reads as an error across the whole panel. The Run guard's popup already reports a clash, so the label stays in the standard text color. The popup now also asks the user to run the data collection again after the run number bump. Co-Authored-By: Claude Fable 5.1 --- src/aare/gui/panels/file_path_panel.py | 11 ++++------- tests/unit/gui/test_data_collection_settings.py | 9 +++------ 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/aare/gui/panels/file_path_panel.py b/src/aare/gui/panels/file_path_panel.py index f897c9b7..f6ede787 100644 --- a/src/aare/gui/panels/file_path_panel.py +++ b/src/aare/gui/panels/file_path_panel.py @@ -7,7 +7,6 @@ from aarecommon.models.models import DAQStatusModel, SampleShortInfo from PySide6.QtCore import Qt, Signal, Slot from PySide6.QtWidgets import QGridLayout, QLabel, QLineEdit, QMessageBox, QSpinBox, QWidget -from aare.gui.styles import PATH_WARN_TEXT from aare.gui.widgets.title_label import TitleLabel ## Logic for filenames: @@ -165,13 +164,11 @@ class FilePathPanel(QWidget): # Store the GUI’s base (without applying scan-kind transforms) for wiring into requests later self._filename = self._expand_macros(base, run_number) - # Preview label shows the effective path (what will be written) + # Preview label shows the effective path (what will be written), in the + # plain theme color: a clash is reported by the Run guard's popup, not + # by a red label. effective = self._effective_dataset_base(self._filename) - exists = self._exists_for_run(self._filename) self.file_name_label.setText(effective + "_master.h5") - # Empty stylesheet = reset to the THEME text color (a hardcoded - # "default" black would be invisible on the dark theme). - self.file_name_label.setStyleSheet(f"color: {PATH_WARN_TEXT};" if exists else "") self.path_updated.emit(self._filename) @Slot() @@ -257,6 +254,6 @@ class FilePathPanel(QWidget): self, "File exists", f"File already exists:\n{path}\n\n" - f"Run number increased to {self.run_number_edit.value()}.", + f"Run number increased to {self.run_number_edit.value()}. Please run data collection again.", ) return False diff --git a/tests/unit/gui/test_data_collection_settings.py b/tests/unit/gui/test_data_collection_settings.py index 1308b9a9..252566c5 100644 --- a/tests/unit/gui/test_data_collection_settings.py +++ b/tests/unit/gui/test_data_collection_settings.py @@ -518,11 +518,9 @@ def test_run_is_blocked_with_a_popup_when_the_file_exists(file_panel, monkeypatc boxes = [] monkeypatch.setattr(QMessageBox, "warning", lambda *a, **k: boxes.append(a)) - # Editing never skips a taken run silently: the name stays literal and - # the label only turns red. + # Editing never skips a taken run silently: the name stays literal. file_panel.set_scan_kind("rotation") assert file_panel.run_number_edit.value() == 1 - assert file_panel.file_name_label.styleSheet() != "" assert file_panel.file_path_error_box(scan_kind="rotation") is False assert len(boxes) == 1 @@ -555,11 +553,10 @@ def test_a_derived_dataset_counts_as_taken(file_panel, monkeypatch): def test_a_directory_or_a_sibling_run_is_not_taken(file_panel, monkeypatch): - # Only the exact master files count; anything else would paint runs red - # that nothing will write. + # Only the exact master files count; anything else would block runs that + # nothing will write. _taken(monkeypatch, "raster/d/x_001", "raster/d/x_0011_master.h5", "raster/d/x_001_data.h5") file_panel.set_scan_kind("raster") - assert file_panel.file_name_label.styleSheet() == "" assert file_panel.file_path_error_box(scan_kind="raster") is True -- 2.54.0 From 90f653386adbf5a9e5a24fb7fc9ba8e79cf78660 Mon Sep 17 00:00:00 2001 From: Dawn Date: Thu, 17 Sep 2026 21:10:58 +0200 Subject: [PATCH 9/9] fix(gui): app-level event filters tolerate PySide's stray QWidgetItem CI (3.12, PR 232) failed test_font_zoom_keeps_frozen_column_aligned_and_reautosizes with a pytest-qt CALL ERROR: PySide 6.9 handed both app-level filters a QWidgetItem instead of a QEvent during layout teardown, and event.type() raised. The wheel guard now keys on isinstance(event, QWheelEvent) and returns False itself instead of super().eventFilter() (which type-checks its arguments and would raise the same way); the cursor filter ignores anything that is not a QEvent. Regression test for each. Co-Authored-By: Claude Fable 5.1 --- src/aare/gui/main_window.py | 4 ++++ src/aare/gui/widgets/wheel_value_guard.py | 11 ++++++++--- tests/unit/gui/test_main_window.py | 11 +++++++++-- tests/unit/gui/test_wheel_value_guard.py | 18 +++++++++++++++++- 4 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/aare/gui/main_window.py b/src/aare/gui/main_window.py index 87f0621d..67fecf58 100644 --- a/src/aare/gui/main_window.py +++ b/src/aare/gui/main_window.py @@ -150,6 +150,10 @@ class ClickableCursorFilter(QObject): forbidden cursor) still win: they set it later.""" def eventFilter(self, obj, event): + # Same PySide 6.9 quirk as WheelValueGuard: an app-level filter can be + # handed a QWidgetItem instead of a QEvent during layout teardown. + if not isinstance(event, QEvent): + return False if event.type() == QEvent.Type.Polish and isinstance( obj, (QAbstractButton, QComboBox, QSlider) ): diff --git a/src/aare/gui/widgets/wheel_value_guard.py b/src/aare/gui/widgets/wheel_value_guard.py index 7697de8e..f06eb504 100644 --- a/src/aare/gui/widgets/wheel_value_guard.py +++ b/src/aare/gui/widgets/wheel_value_guard.py @@ -1,4 +1,4 @@ -from PySide6.QtCore import QEvent, QObject, Qt +from PySide6.QtCore import QObject, Qt from PySide6.QtGui import QWheelEvent from PySide6.QtWidgets import ( QAbstractScrollArea, @@ -26,7 +26,10 @@ class WheelValueGuard(QObject): GUARDED = (QAbstractSpinBox, QSlider, QDial, QComboBox, QTabBar) def eventFilter(self, obj, event): - if event.type() == QEvent.Type.Wheel and isinstance(obj, self.GUARDED): + # isinstance, not event.type(): PySide 6.9 sometimes hands an app-level + # filter a QWidgetItem instead of a QEvent while layouts are torn down, + # and .type() then raises (pytest-qt CALL ERROR on a random test). + if isinstance(event, QWheelEvent) and isinstance(obj, self.GUARDED): if event.buttons() & Qt.MouseButton.RightButton: return False # right button held: deliberate value adjustment area = obj.parentWidget() @@ -45,7 +48,9 @@ class WheelValueGuard(QObject): ) QApplication.sendEvent(area.viewport(), relayed) return True - return super().eventFilter(obj, event) + # Not super().eventFilter(): QObject's does nothing but type-checks its + # arguments, so the stray QWidgetItem would raise there instead. + return False if __name__ == "__main__": diff --git a/tests/unit/gui/test_main_window.py b/tests/unit/gui/test_main_window.py index c7f2e3ab..c6cba4c0 100644 --- a/tests/unit/gui/test_main_window.py +++ b/tests/unit/gui/test_main_window.py @@ -3,10 +3,10 @@ from unittest.mock import MagicMock, patch import pytest from PySide6.QtCore import QSettings, Qt -from PySide6.QtWidgets import QApplication, QDockWidget +from PySide6.QtWidgets import QApplication, QDockWidget, QWidget, QWidgetItem from aare.gui import styles -from aare.gui.main_window import MainWindow +from aare.gui.main_window import ClickableCursorFilter, MainWindow from aare.gui.styles import THEME_BLUEBIRD, THEME_SUNRISE, THEME_SUNSET @@ -683,3 +683,10 @@ def test_sample_camera_frame_paints_visible_views_and_acks(qtbot, mock_ui_state, # No subscriber (GUI started without a sample feed): the slot must not blow up. win.prediction_thread = None win._on_sample_camera_frame(QImage(4, 6, QImage.Format.Format_RGB888)) + + +def test_cursor_filter_ignores_a_non_event_argument(qapp): + # PySide 6.9 can hand an app-level filter a QWidgetItem instead of a + # QEvent during layout teardown; raising there fails a random test. + widget = QWidget() + assert ClickableCursorFilter().eventFilter(widget, QWidgetItem(widget)) is False diff --git a/tests/unit/gui/test_wheel_value_guard.py b/tests/unit/gui/test_wheel_value_guard.py index 68d760a9..30ae245c 100644 --- a/tests/unit/gui/test_wheel_value_guard.py +++ b/tests/unit/gui/test_wheel_value_guard.py @@ -5,7 +5,15 @@ deliberate right-button + wheel gesture.""" import pytest from PySide6.QtCore import QPoint, QPointF, Qt from PySide6.QtGui import QWheelEvent -from PySide6.QtWidgets import QApplication, QScrollArea, QSlider, QSpinBox, QVBoxLayout, QWidget +from PySide6.QtWidgets import ( + QApplication, + QScrollArea, + QSlider, + QSpinBox, + QVBoxLayout, + QWidget, + QWidgetItem, +) from aare.gui.widgets.wheel_value_guard import WheelValueGuard @@ -72,3 +80,11 @@ def test_bare_wheel_scrolls_the_enclosing_area(guard, qtbot): QApplication.sendEvent(spin, _wheel(Qt.MouseButton.NoButton)) assert spin.value() == 50 assert bar.value() != before + + +def test_a_non_event_argument_is_ignored(guard, qtbot): + # PySide 6.9 can hand an app-level filter a QWidgetItem instead of a + # QEvent during layout teardown; raising there fails a random test. + widget = QWidget() + qtbot.addWidget(widget) + assert guard.eventFilter(widget, QWidgetItem(widget)) is False -- 2.54.0