pyapp-kit / psygnal

Python observer pattern (callback/event system). Modeled after Qt Signals & Slots (but independent of Qt)
https://psygnal.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
84 stars 13 forks source link

feat: Add recursion_mode ('immediate' or 'deferred') to Signal and SignalInstance #293

Closed tlambert03 closed 7 months ago

tlambert03 commented 7 months ago

fixes #292, and represents a partial reversion of #281 (the behavior of #281 is still possible, but must be opted into using recursion_mode = 'deferred')

This adds the possibility to control the order callbacks during recursive emission events (when one of the callbacks itself emits the same event):

@Czaki, thoughts welcome on parameter naming and signature, etc... (i.e. should it be a boolean recurse_immediately instead of a set of literal strings? will we ever want additional modes?)

codecov[bot] commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (c643ad0) to head (1589cec).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #293 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 22 22 Lines 1933 1973 +40 ========================================= + Hits 1933 1973 +40 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codspeed-hq[bot] commented 7 months ago

CodSpeed Performance Report

Merging #293 will improve performances by 10.36%

Comparing tlambert03:recursion-mode (1589cec) with main (c643ad0)

Summary

⚡ 1 improvements ✅ 65 untouched benchmarks

Benchmarks breakdown

Benchmark main tlambert03:recursion-mode Change
test_emit_time[setattr-50] 225.7 µs 204.5 µs +10.36%
tlambert03 commented 7 months ago

The failing benchmarks represent a slight increase in the time to create a signal instance:

Change Before [f4ac7c3f] <v0.10.0^0> After [b17be85c] Ratio Benchmark (Parameter)
+ 477±3ns 833±3ns 1.74 benchmarks.CreateSuite.time_create_signal_instance
+ 4.80±0.05μs 5.87±0.08μs 1.22 benchmarks.EventedSetSuite.time_create_set(10)
+ 4.85±0.03μs 5.92±0.02μs 1.22 benchmarks.EventedSetWithCallbackSuite.time_create_set(10)

which is not something I'm worried about here given that it's on the order of ns.

@Czaki, this is ready for final review. I'll wait for your review on this one before merging. And I intend to follow up in another PR reviewing the exception handling logic slightly