feat: add thread parameter to connection method, allowed "queued connections"

[tlambert03]

alternative to #199 ... hat-tip to @Czaki for the inspiration.

somewhat closer to realizing #198

run this script as an example

import threading
from qtpy.QtWidgets import QApplication
from psygnal import Signal
# special qt support, so everyone can share the same main-thread timer
from psygnal.qt import start_emitting_from_queue 


class Emitter:
    sig = Signal(int)

obj = Emitter()

@obj.sig.connect(thread='main')
def on_emit(value: int) -> None:
    """Callback function that states its thread."""
    print(f"got value {value} from thread {threading.current_thread().name!r}")

app = QApplication([])
start_emitting_from_queue()
threading.Thread(target=obj.sig.emit, args=(1,)).start()
app.processEvents()

[codspeed-hq]

CodSpeed Performance Report

Merging #200 tlambert03:connect-queue (1b2adf7) will not alter performances.

Summary

🔥 0 improvements
❌ 0 regressions
✅ 66 untouched benchmarks

🆕 0 new benchmarks
⁉️ 0 dropped benchmarks

[codecov]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (74aa34f) 100.00% compared to head (1b2adf7) 100.00%.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #200   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           18        21    +3     
  Lines         1571      1630   +59     
=========================================
+ Hits          1571      1630   +59     

Impacted Files	Coverage Δ
src/psygnal/__init__.py	100.00% <100.00%> (ø)
src/psygnal/_exceptions.py	100.00% <100.00%> (ø)
src/psygnal/_queue.py	100.00% <100.00%> (ø)
src/psygnal/_signal.py	100.00% <100.00%> (ø)
src/psygnal/qt.py	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[Czaki]

I try to suggest an extension of this to provide a queue per thread. What did you think? Maybe weakkeydict should be used.

[tlambert03]

Thanks! Will look at this closer tomorrow. Do you generally like this pattern better on the whole?

[Czaki]

Yes. I prefer it as the user decides if use queued connections.

[Czaki]

there was bug in my comments

[tlambert03]

question regarding API on the connect method @Czaki ...

I find I don't love the name type (other than the fact that Qt uses the name type). One alternative would be to use

def connect(self, ..., thread: Thread | None = None): ...

where thread is whatever thread you want this callback to be invoked in.

• thread=None would mean invoke this slot on whatever thread the signal was emitted from.
• threading=threading.main_thread() or threading=threading.current_thread() does what it sounds like.

we could also accept the special strings 'main' or 'current' as conveniences to avoid needing to import from threading.

or do you prefer the words "direct" and "queued"?

[Czaki]

I like this suggestion to pass thread as primary type.

[tlambert03]

I like this suggestion to pass thread as primary type.

so, no conveniences around string? user must import from threading in order to use it?

[Czaki]

Yes. Support for main and current is a good idea. As we do not have an idea for moving object semantics, then it is better than direct and queued.

[tlambert03]

one more question. I'm inclined to default to the following behavior (most similar to "auto connection" on Qt):

• if connect(callback, thread=some_thread) is used, then when a signal is emitted on some_thread, then callback is invoked immediately (rather than being queued for later callback).

This avoids the surprising behavior of using connect(callback, thread=main_thread()) in a single-threaded program, and then never seeing callback get called (unless you then specifically went in and used emit_queued(main_thread())).

in code, the QueuedCallback object would then look like this:

class QueuedCallback(WeakCallback):
    def cb(self, args: tuple = ()) -> None:
        if current_thread() is self._thread:
            self._wrapped.cb(args)
        else:
            QueuedCallback._GLOBAL_QUEUE[self._thread].put((self._wrapped.cb, args))

do you agree?

[Czaki]

Hm. It looks like the best solution for now.

[Czaki]

maybe also https://github.com/pyapp-kit/psygnal/blob/main/docs/usage.md should be updated?

[tlambert03]

maybe also https://github.com/pyapp-kit/psygnal/blob/main/docs/usage.md should be updated?

i'll do that in a follow up. I think i will also deprecate the asynchronous parameter on the emit method. Someone can always just manually emit in a different thread. The more important thing was always where the callback gets called and this solves that much better.

[tlambert03]

excited about this! thanks a lot for your input @Czaki