E530 refactor: split out SlotCaller logic into new `weak_callable` module... maybe public eventually by tlambert03 · Pull Request #178 · pyapp-kit/psygnal · GitHub
[go: up one dir, main page]
Skip to content

refactor: split out SlotCaller logic into new weak_callable module... maybe public eventually#178

Merged
tlambert03 merged 23 commits intopyapp-kit:mainfrom
tlambert03:weak-callback
Feb 20, 2023
Merged

refactor: split out SlotCaller logic into new weak_callable module... maybe public eventually#178
tlambert03 merged 23 commits intopyapp-kit:mainfrom
tlambert03:weak-callback

Conversation

@tlambert03
Copy link
Member
@tlambert03 tlambert03 commented Feb 19, 2023 •
edited
Loading

This PR moves all of the "_SlotCaller" class logic into a new "_weak_callback" module, and restructures it quite a bit. The idea is that it could potentially be made public. (A good part of the effort that goes into psygnal is in ensuring that all of the connected callbacks are both as weakly referenced as possible, in many edge cases, while also being as fast as possible to deference in call at emit-time ... and that logic lives independently enough)

the psygnal._weak_callback.weak_callback function is the primary factory_function now, and the cb method is how it should be called.

from psygnal._weak_callback import weak_callback

class T:
    def greet(self, name):
        print("hello,", name)

def _on_delete(weak_cb):
    print("deleting!")

t = T()
weak_cb = weak_callback(t.greet, finalize=_on_delete)

weak_cb.cb(("world",))  # "hello, world"

del t  # "deleting!"

weak_cb.cb(("world",))  # ReferenceError

This PR significantly increases the time for the connect_setattr and connect_setitem, but those are relatively rarely used functions, that only get called once. In turn, this PR slightly improves the emission time for most callback types (which is the much more performance critical part) and I think makes for a much cleaner code pattern

@codspeed-hq
Copy link
codspeed-hq bot commented Feb 19, 2023 •
edited
Loading

CodSpeed Performance Report

Merging #178 weak-callback (6321b49) will degrade performances by 10.0%.

Summary

🔥 0 improvements
❌ 4 (👁 4) regressions
✅ 54 untouched benchmarks

🆕 0 new benchmarks
⁉️ 0 dropped benchmarks

Benchmarks breakdown

Benchmark main weak-callback Change
👁 test_connect_time[setattr-check_types] 35.1 µs 82 µs -133.46%
👁 test_connect_time[setattr-] 35.1 µs 82.7 µs -135.32%
👁 test_connect_time[setitem-check_types] 36.2 µs 82.5 µs -128.00%
👁 test_connect_time[setitem-] 36.3 µs 84.2 µs -132.35%

@tlambert03 tlambert03 changed the title feat: [WIP] Weak callback module refactor: [WIP] Weak callback module Feb 19, 2023
@codecov
Copy link
codecov bot commented Feb 19, 2023 •
edited
Loading

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (6321b49) compared to base (cfdd6a6).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files 
@@            Coverage Diff            @@
##              main      #178   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           15        16    +1     
  Lines         1451      1495   +44     
=========================================
+ Hits          1451      1495   +44
Impacted Files Coverage Δ
src/psygnal/_signal.py 100.00% <100.00%> (ø)
src/psygnal/_weak_callback.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 at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tlambert03 tlambert03 marked this pull request as ready for review February 19, 2023 21:10
@tlambert03 tlambert03 mentioned this pull request Feb 19, 2023
Closed
@tlambert03 tlambert03 changed the title refactor: [WIP] Weak callback module refactor: split out SlotCaller logic into new weak_callable module... maybe public eventually Feb 19, 2023
@tlambert03 tlambert03 mentioned this pull request Feb 20, 2023
Merged
@tlambert03 tlambert03 merged commit 186ec57 into pyapp-kit:main Feb 20, 2023
@tlambert03 tlambert03 deleted the weak-callback branch February 20, 2023 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

refactor

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@tlambert03
0