Black magic #46
Black magic #46
Conversation
The boundevent is now cached as attribute of the instance and never gets garbage collected before its owner object. The list of event handlers becomes an instance variable of the boundevent object. This is reasonable since it is (and always was) the boundevent object's responsibility to manage the event handlers. Furthermore, this makes the boundevent object more self-contained (independent of the instance it is attached to), which will make it usable on its own.
Together with the previous commit (making the boundevent class more independent), this will allow the following useful reinterpretation: - boundevent becomes an independent event class (the main event class even) that is fully capable exist without being an attribute within another class. - the event class is just a helper to dynamically associate boundevent objects to objects. It can be viewed as a form of a '@cachedproperty' descriptor utility.
This enables boundevent to use any externally created list (but it will still add and remove handlers to it!).
The caching of the boundevent instance will have the effect that it the reference count of the object which it is tied to can never reach zero. It can be cleaned up only by the garbage collector. This may be very undesirable (and unexpected) in some circumstances. Instead, we can use the event_handler argument of boundevent to create boundevents that are specific to some instance.
Note, that this also makes the overload of __set__ unnecessary.
Conflicts: obsub.py test/test_core.py test/test_weakref.py
The new 'signal' can be used independently and deserves a better name than 'boundevent'!
This makes the code much easier to understand. Furthermore, the closure namespace that contains the function and event_handlers makes underscore prefixes for "private" variables unnecessary. Furthermore, this will make it easy to use 'black_magic.decorator' with this module.
Hey, since we are writing decorator code here, why not do stuff that is only for really cool people?
The docstring of the wrapper closure is hidden from the outside world, so it makes no sense. Furthermore, the @wraps decorator now takes care to ensure the correct call signature, contrary to what the removed comment wants to make us believe.
For what it's worth this makes class-based access very slightly faster, since black_magic.decorator.wraps has to be invoked only once per attribute access. Huge use case... ;)
Okay, there are two relevant changes: - remove the partial() stuff - remove doctests for memory management These are just confusing to read and a user wouldn't even get the idea that these fail unless explicitly stated - they are not documentation but tests.
I forgot that every function is a descriptor, which makes it very convenient to create bound member functions.
The event descriptor object is usually not inspected and doesn't need to be updated. Even if it were inspected, it is questionable to update its documentation.
|
Great work! This is a major rewrite, so it'll take some time to review. Regarding the black_magic dependency: it does preserve function signatures, so this is probably important functionality beyond documentation, is it not? Otherwise, could you reasonably strip it out of this PR? I'm not saying that you should, just trying to assess our options here. |
This allows the obsub module to be used as a drop-in more easily. On the other hand this can have unexpected results when using functions with default arguments without having black-magic installed: You need to make sure, that all parameters that have default values in your original event function have default values in all handlers as well!
|
It is possible to exchange |
|
Btw., c5b00ff (Use function.get instead of black_magic.partial) should restore approximately the original speed but makes it much harder to decorate anything but basic functions. So, if we care about covering many use cases, we should probably revert this. |
|
Finally, I managed to do some rudimentary speed tests.. Turns out, that The code in the latest change may seem a little confusing, but it mostly fixes the performance issue by moving the only If you are interested in the benchmark: from obsub import event
import timeit
def create():
class X(object):
@event
def foo(self, bar=3):
return bar
return X
X = create()
x = X()
def invoke():
x.foo()
x.foo(2)
def access():
X.foo
def bench(func, number=10000):
return timeit.timeit("%s()" % func,
setup="from __main__ import %s" % func,
number=number)
if __name__ == '__main__':
print(bench('create'))
print(bench('invoke'))
print(bench('access'))Result on CPython2.7
This looks similar onCPython2.6 and CPython3.3. On pypy, however, the optimization doesn't make much difference:
|
1 similar comment
In the interest of a more descriptive name that shows the relation to the standard `event`. Possibly, `event` should be renamed as well. For example to `instance_event` or `member_event`.
The following signatures are removed:
cls.event.connect(instance, handler)
The following stays valid:
cls.event.connect(handler)
inst.event.connect(handler)
The rationale is simplicity: It is easier to remember a single calling
signature with only one argument. In particular the removed signature
might be confusing in terms of parameter order, since the *first*
parameter was optional.
Furthermore, this is easier to handle internally, especially with regard
to upcoming revival of a bound_event class.
|
There is a fatal flaw in this branch: Assuming Reason: The only possibility I can see to get the desired behaviour, is to revert back to a |
The previous approach was returning a new copy of a function from `event.__get__` each time. Since function comparison is done by identity this breaks code like `.connect(a.foo)` followed by `.disconnect(a.foo)` (assuming `a.foo` is an event). Furthermore, in python it is not possible to overload equality of functions (it is not possible to derive from FunctionType/MethodType). Therefore, the only possibility I can see to get the desired behaviour, is to revert back to a bound_event class.
To enable later access of __get__ like in:
- foo.bar.__get__(...)
- Foo.bar.__get__(foo)
This is more flexible than a boolean option _decorate=True.
To show function signature in help strings
I finally managed to create a reasonable version that uses
black_magic.decoratorto create nice function objects that preserve parameter names and default arguments even on python2.Possible issues to take into consideration: