MDA position list is silently zeroed when switching objective #562

Storing positions in the MDA widget's position list and then switching objective resets every stored position to x=0, y=0, z=0. Only happens when the config file has pixel size calibrations, because that is what makes an objective switch emit pixelSizeChanged, and that signal triggers the code path that fails.

Digging into it I think I found multiple (interacting) issues with the widget, I list them all here and would work towards 3 independent PRs if you agree, have some open questions though. Minimal repro at the bottom.

1: GUI code runs on the wrong thread (causes the data loss)

Problem: The position table rebuilds itself whenever the pixel size changes. That handler is connected directly to core events. When the core uses the psygnal signal backend (which happens silently whenever CMMCorePlus.instance() is created before the QApplication, common in notebooks), core events arrive on a background thread. The rebuild then races against Qt's queued row creation: the real values are written first and the empty default cells (0.0) are created after, so every position ends up at the origin. Creating the QApplication before the core makes the same script pass.

Fix: Decorate the handler with superqt.ensure_main_thread so it always runs on the GUI thread. The repo already uses this decorator in the stack viewer for the same reason. Verified this one-line change fixes the repro.

2: the pixel size handler rebuilds the whole table and loses data (even on the correct thread)

Problem: To update the field-of-view size on grid sub-sequences, the handler reads the whole table with value() and writes it back with setValue(). That round trip is lossy: hidden columns come back as 0 (e.g. z is reset on every objective switch when "include z" is unchecked), unchecked rows are deleted, and an HCS plate plan is flattened to plain positions.

Fix: Do not rebuild the table. Update only the positions that actually have a grid plan, and return early when none do. Note this alone does not fix Bug 1: as soon as a position carries a grid plan the table is written again and the thread race comes back.

3: core signal connections are leaked on widget destruction

Problem: The core-connected grid widget never disconnects its core signals, and the position table disconnects only two of its four. Every MDA widget that gets created and destroyed leaves dead callbacks behind. Once the underlying Qt object is gone these raise RuntimeError: wrapped C/C++ object ... has been deleted on every pixel size change, and with psygnal one raising callback prevents the remaining callbacks from running.

Fix: Disconnect everything that was connected: add a _disconnect to the grid widget and the two missing lines to the position table's.

Status

Bug 1 is not specific to the position table. Under the psygnal backend every core-event handler runs on the core callback thread, and an audit of src/ finds 81 of 86 connected handlers updating Qt widgets there. One of them writes back to the hardware: PropertyWidget._on_core_changed sets its slider/spinbox under blockSignals, but the inner widgets' valueChanged signals reach the main thread after the block is released, and _on_widget_changed calls setProperty. Measured on a microscope with the Device Property Browser open during an acquisition: several hundred setProperty calls per run from the GUI thread. Reproducible with the demo config. #563 fixes that widget and ExposureWidget with ensure_main_thread; the same treatment for the position table (and the rest) is what question (a) is about.

Bugs 2 and 3 are unchanged.

Open questions

a. Should ensure_main_thread be applied to all core-event handlers in the core-connected widgets, or only where breakage is demonstrated? Under psygnal, nearly all core events arrive on a background thread, so the exposure is broad.
b. What is the intended behavior of the pixel size handler: may it delete unchecked rows, reset z, or flatten an HCS plan? I guess it's not intended, but would change behavior.
c. Add a warning when a psygnal-backed core drives Qt widgets? Ran into other issues many times, I think a this could help with troubleshooting.

Repro

import gc
import threading
from pathlib import Path

import useq
from pymmcore_plus import CMMCorePlus

mmc = CMMCorePlus.instance()  # created BEFORE QApplication -> psygnal backend
from qtpy.QtWidgets import QApplication

app = QApplication([])
from pymmcore_widgets.mda import MDAWidget

CFG = Path(__file__).parent / "tests" / "test_config.cfg"
mmc.loadSystemConfiguration(str(CFG))

from importlib.metadata import version

print("signaler:", type(mmc.events).__name__, "| pymmcore", version("pymmcore"))
# NOTE: bug 1 needs pymmcore >= 12 (MMCore 12 delivers callbacks on a dedicated
# thread; 11.x delivers them on the calling thread, so the switch below stays on
# MainThread and the positions survive)

POS = [useq.Position(x=100, y=200, z=50), useq.Position(x=-300, y=400, z=50)]

wdg = MDAWidget(mmcore=mmc)
wdg.tab_wdg.setChecked(wdg.stage_positions, True)
pt = wdg.stage_positions

# --- Bug 1: objective switch zeroes x/y/z (handler runs on a background thread) ---
print("\n--- Bug 1: objective switch under psygnal ---")
mmc.events.pixelSizeChanged.connect(
    lambda v: print("  pixelSizeChanged on thread:", threading.current_thread().name)
)
pt.include_z.setChecked(True)
pt.setValue(POS)
print("  before:", [(p.x, p.y, p.z) for p in pt.value()])
mmc.setConfig("Objective", "40X")
mmc.waitForSystem()
for _ in range(3):
    app.processEvents()
after = [(p.x, p.y, p.z) for p in pt.value()]
print("  after :", after)
print("  -> BUG 1 CONFIRMED" if all(a[:2] == (0.0, 0.0) for a in after) else "  -> not reproduced")

# --- Bug 2: lossy round trip wipes z, no threading involved ---
print("\n--- Bug 2: direct _update_fov_size() call on the main thread ---")
pt.setValue(POS)
pt.include_z.setChecked(False)  # default state: z column hidden
pt._update_fov_size()           # plain call, main thread
pt.include_z.setChecked(True)
after = [(p.x, p.y, p.z) for p in pt.value()]
print("  after :", after)
print("  -> BUG 2 CONFIRMED (z lost)" if all(a[2] in (0.0, None) for a in after) else "  -> not reproduced")

# --- Bug 3: connections leak when widgets are destroyed ---
print("\n--- Bug 3: leaked core signal connections ---")
n0 = len(mmc.events.pixelSizeChanged)
extra = [MDAWidget(mmcore=mmc) for _ in range(2)]
n1 = len(mmc.events.pixelSizeChanged)
for w in extra:
    w.close()
    w.deleteLater()
app.processEvents()
gc.collect()
app.processEvents()
n2 = len(mmc.events.pixelSizeChanged)
print(f"  callbacks: {n0} -> {n1} after creating 2 widgets -> {n2} after deleting them")
print("  -> BUG 3 CONFIRMED (leaked)" if n2 > n0 else "  -> not reproduced")

Tested on main (d61a158), pymmcore-plus 0.18.1, pymmcore 12.5.0.75.0, PyQt6 6.11.0, Python 3.11, macOS.
Requires pymmcore >= 12, where MMCore delivers callbacks on a dedicated thread (11.x delivers them on the calling thread, which hides the bug in demo setups