Skip to content

Commit cb8fdf3

Browse files
committed
Fix Event.emit dispatch bugs: empty kwargs list and positional arg duplication
Two bugs fixed: 1. connect(callback, []) with empty list silently dropped callbacks instead of connecting a no-argument wrapper. The len(kwargs) > 0 guard skipped connection entirely. 2. In emit: positional args were incorrectly duplicated as keyword args for native psygnal callbacks, causing TypeError in callbacks like WidgetBase.disconnect() which accept *args but not **kwargs. Fixed by dispatching native callbacks with raw positional+keyword args and wrapper callbacks with keyword-only args. Additionally, single positional emit args are now mapped to 'obj' keyword for wrapper-based callbacks, matching the universal HyperSpy convention. Add regression tests for Event.emit dispatch edge cases - Empty kwargs list with no-arg wrapper - Positional emit args not duplicated as keywords for native callbacks - Positional emit args mapped to 'obj' keyword for wrapped callbacks Assisted-by: opencode/glm-5.2
1 parent c996c8c commit cb8fdf3

2 files changed

Lines changed: 117 additions & 17 deletions

File tree

hyperspy/events.py

Lines changed: 56 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -236,10 +236,19 @@ def emit(self, *args, **kwargs):
236236
``_arguments``, if set) — matching the :meth:`trigger` behaviour
237237
for backward compatibility with code like ``emit(signal)``.
238238
"""
239-
# Map positional args to declared argument names (same as trigger())
240-
if args and self._arguments:
241-
for name, val in zip(self._arguments, args, strict=True):
242-
kwargs.setdefault(name, val)
239+
# Map positional args to keyword names for wrapper-based dispatch.
240+
# "all"-mode callbacks (native psygnal) receive args as positional;
241+
# "some"/"map" wrapper callbacks (lambda **kw) need every arg as keyword.
242+
wrapper_kwargs = dict(kwargs)
243+
if args:
244+
if self._arguments:
245+
for name, val in zip(self._arguments, args, strict=True):
246+
wrapper_kwargs.setdefault(name, val)
247+
elif len(args) == 1:
248+
wrapper_kwargs.setdefault("obj", args[0])
249+
else:
250+
for i, val in enumerate(args):
251+
wrapper_kwargs[f"_arg{i}"] = val
243252

244253
if self._is_blocked or self._suppress:
245254
return
@@ -253,7 +262,7 @@ def emit(self, *args, **kwargs):
253262

254263
# Debounce guard: defer emission, resetting timer on each call
255264
if self._debounce_interval is not None:
256-
self._debounce_pending_kwargs = kwargs
265+
self._debounce_pending_kwargs = wrapper_kwargs
257266
if self._debounce_timer is not None:
258267
self._debounce_timer.cancel()
259268
self._debounce_timer = threading.Timer(
@@ -262,12 +271,28 @@ def emit(self, *args, **kwargs):
262271
self._debounce_timer.start()
263272
return
264273

265-
self._emit_dispatch(args, kwargs)
274+
self._emit_dispatch(args, kwargs, wrapper_kwargs)
266275

267-
def _emit_dispatch(self, args, kwargs):
276+
def _emit_dispatch(self, args, kwargs, wrapper_kwargs):
268277
"""Core dispatch: validate, order, and invoke callbacks."""
278+
# Determine which kwarg names came from positional arg mapping.
279+
# These must NOT be passed as keywords to "all"-mode callbacks,
280+
# since they're already passed positionally via *args.
281+
pos_kwarg_names = set()
282+
if args:
283+
if self._arguments:
284+
pos_kwarg_names = set(self._arguments[: len(args)])
285+
elif len(args) == 1:
286+
pos_kwarg_names = {"obj"}
287+
else:
288+
pos_kwarg_names = {f"_arg{i}" for i in range(len(args))}
289+
269290
if self._arguments:
270-
kwargs = self._validate_emit_kwargs(kwargs)
291+
wrapper_kwargs = self._validate_emit_kwargs(wrapper_kwargs)
292+
# Apply defaults back to the "all"-mode kwargs
293+
for k, v in wrapper_kwargs.items():
294+
if k not in pos_kwarg_names:
295+
kwargs.setdefault(k, v)
271296

272297
# Snapshot slots so connect/disconnect during dispatch are safe.
273298
# Dispatch in legacy order: "all" → "some" → "map". This
@@ -290,19 +315,25 @@ def _emit_dispatch(self, args, kwargs):
290315
else: # "map"
291316
map_callbacks.append(callback)
292317

293-
for callback in all_callbacks + some_callbacks + map_callbacks:
318+
for callback in all_callbacks:
294319
original = self._find_original(callback)
295320
if original in self._suppressed_callbacks:
296321
continue
297322
callback(*args, **kwargs)
298323

324+
for callback in some_callbacks + map_callbacks:
325+
original = self._find_original(callback)
326+
if original in self._suppressed_callbacks:
327+
continue
328+
callback(**wrapper_kwargs)
329+
299330
def _debounce_fire(self):
300331
"""Called by the debounce timer — fires the pending emission."""
301332
kwargs = self._debounce_pending_kwargs
302333
self._debounce_pending_kwargs = None
303334
self._debounce_timer = None
304335
if kwargs is not None:
305-
self._emit_dispatch(kwargs)
336+
self._emit_dispatch((), {}, kwargs)
306337

307338
@contextmanager
308339
def throttle(self, interval):
@@ -458,14 +489,22 @@ def connect(self, function, kwargs="all", **psygnal_opts):
458489
self._slot_mode[wrapper] = "map"
459490

460491
elif isinstance(kwargs, (list, tuple)):
461-
# only make a wrapper when there are argument to pass
462-
if len(kwargs) > 0:
463-
spec = tuple(kwargs)
492+
spec = tuple(kwargs)
493+
if len(spec) > 0:
464494
wrapper = self._make_list_wrapper(function, spec)
465-
super().connect(wrapper, **psygnal_opts)
466-
self._wrapper_map[function] = (wrapper, spec)
467-
self._connected_originals.add(function)
468-
self._slot_mode[wrapper] = "some"
495+
else:
496+
# Empty → pass no kwargs (wrapper discards all emit args)
497+
def _make_empty_wrapper(fn):
498+
def _empty_wrapper(**kw):
499+
return fn()
500+
501+
return _empty_wrapper
502+
503+
wrapper = _make_empty_wrapper(function)
504+
super().connect(wrapper, **psygnal_opts)
505+
self._wrapper_map[function] = (wrapper, spec)
506+
self._connected_originals.add(function)
507+
self._slot_mode[wrapper] = "some"
469508

470509
else:
471510
raise ValueError("Invalid value passed to kwargs.")

hyperspy/tests/test_events.py

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,67 @@ def lambda2(x=None, y=None, A=None, B=None):
455455
self.events.a.connect(lambda2)
456456
self.events.a.trigger(A="vA", B="vB")
457457

458+
def test_connect_empty_kwargs_connects(self):
459+
# Regression: connect(callback, []) must connect a wrapper that
460+
# calls callback with no arguments. Previously, len(kwargs) > 0
461+
# skipped the connection silently, so the callback never fired.
462+
called = []
463+
464+
def callback_no_args():
465+
called.append(True)
466+
467+
with pytest.warns(VisibleDeprecationWarning):
468+
# Deprecated API — Event() and connect(kwargs=[]) both emit
469+
e = Event(arguments=["A", "B"])
470+
e.connect(callback_no_args, [])
471+
472+
with pytest.warns(VisibleDeprecationWarning):
473+
# Deprecated API — trigger() emits
474+
e.trigger(A="vA", B="vB")
475+
476+
assert len(called) == 1
477+
478+
def test_emit_positional_no_kwarg_duplication(self):
479+
# Regression: emit(self) on an EventSignal without explicit
480+
# arguments must pass the positional arg to native callbacks
481+
# only as a positional arg, not also as a keyword "obj".
482+
# Callbacks that accept *args but not **kwargs (like
483+
# WidgetBase.disconnect) used to fail.
484+
# self.events.c has _arguments is None.
485+
e = self.events.c
486+
received = []
487+
488+
def callback_starargs(*args):
489+
received.append(args)
490+
491+
e.connect(callback_starargs)
492+
sentinel = object()
493+
e.emit(sentinel)
494+
assert len(received) == 1
495+
assert received[0][0] is sentinel
496+
assert "obj" not in received[0]
497+
498+
def test_emit_positional_obj_for_wrapped_callback(self):
499+
# When emit(self) is used on an EventSignal without explicit
500+
# arguments, wrapper callbacks linked with connect(..., ["obj"])
501+
# should receive the single positional arg as keyword "obj",
502+
# matching the HyperSpy arg convention for events with
503+
# explicit arguments=["obj"].
504+
e = self.events.c
505+
received = []
506+
507+
def callback_obj(obj):
508+
received.append(obj)
509+
510+
with pytest.warns(VisibleDeprecationWarning):
511+
# Deprecated API — connect(kwargs=["obj"])
512+
e.connect(callback_obj, ["obj"])
513+
514+
sentinel = object()
515+
e.emit(sentinel)
516+
assert len(received) == 1
517+
assert received[0] is sentinel
518+
458519

459520
# ---------------------------------------------------------------------------
460521
# Added regression tests — preserved deprecated behaviours and native API

0 commit comments

Comments
 (0)