Chore: Event callbacks can have order#6080
Conversation
BigRoy
left a comment
There was a problem hiding this comment.
This looks clean, really nice and clear. Nice work!
I do wonder whether it's time to add a pytests for this.
The tests would need to assert that:
- The callbacks run in the order they were registered (if no order specificied)
- That ordering works fine when mixed with unordered (which is default order
0) - That the event system works without errors with partial functions
Something along the lines of:
from functools import partial
from openpype.lib.events import QueuedEventSystem
def test_unordered_events():
"""
Test if pyblish filter can filter and modify plugins on-the-fly.
"""
result = []
def function_a():
result.append("A")
def function_b():
result.append("B")
def function_c():
result.append("C")
# Without order
event_system = QueuedEventSystem()
event_system.add_callback("test", function_a)
event_system.add_callback("test", function_b)
event_system.add_callback("test", function_c)
event_system.emit("test", {}, "test")
assert result == ["A", "B", "C"]
def test_ordered_events():
result = []
def function_a():
result.append("A")
def function_b():
result.append("B")
def function_c():
result.append("C")
def function_d():
result.append("D")
def function_e():
result.append("E")
def function_f():
result.append("F")
# Without order
event_system = QueuedEventSystem()
event_system.add_callback("test", function_a)
event_system.add_callback("test", function_b, order=-10)
event_system.add_callback("test", function_c, order=10)
event_system.add_callback("test", function_d, order=5)
event_system.add_callback("test", function_e)
event_system.add_callback("test", function_f, order=10)
event_system.emit("test", {}, "test")
assert result == ["B", "A", "E", "D", "C", "F"]
def test_events_partial_callbacks():
result = []
def function(name):
result.append(name)
def function_regular():
result.append("regular")
event_system.add_callback("test", partial(function, "foo"))
event_system.add_callback("test", function_regular)
event_system.add_callback("test", partial(function, "bar"))
event_system.emit("test", {}, "test")
assert result == ["foo", "regular", "bar"]Note that I didn't test this PR, sorry - lack of time!
Will add when I have time. Not merging now as the feature is not "must have". |
I understand - however, I'll hold of working on this PR #5830 until this is merged I suppose - to move it to this event system? |
|
Partial are still not working, will be in next PR. Meaning they still have to be kept in memory somewhere. |
If you want - I can also take a look at this PR's branch and try to implement a fix. Thoughts? |
I have it implemented, but that needs more testing and I would like to merge this. |
|
Sounds good to me - looking forward to the other PR then. |
|
Did some testing and it looks like it works, so I've added it to this PR. |
|
One note, if you combine callbacks with and without orders, those without should imho run last? |
Currently they run at order So if you want to run before unordered, then register at a negative order. |
Changelog Description
Event callbacks can have order in which are called, and fixed issue with getting function name and file when using
partialfunction as callback.Additional info
The original idea came from https://github.com/ynput/OpenPype/pull/5830/files#diff-8ac00fa37b696103daef385811bac7aa3d6bb25a55e7fac15dbf5a801b69764aR453 by @BigRoy. It was modified to have the option to define order in existing event system classes.
Difference from that PR is that callbacks are not stored by order in a dictionary, but are stored in a list, and order is an attribute of callback. Callbacks are sorted on each event emit.
I did some speed tests and didn't notice any difference in different approaches of handling the order, all of them were almost identical (even previous implementation is almost matching the speed of current implementation).
Testing notes:
EventSystemorQueuedEventSystem.