Fundamental changes for v0.3

[milibopp]

There are some ideas, how one could improve on the current interface to make it more flexible. This could probably go beyond the current decorator-only implementation. The usage of the -= and += operators is also something to be discussed.

From my point of view it would make sense to collect some use cases, for which this would be necessary. Then we could condense into a more concrete picture of what could change.

[DanielSank]

I have a few comments on this. I tried contacting people through the mailing list but got no response, so I guess it's defunct.

1. The += and -= is quite non-pythonic. I really think the correct interface should be like this: thing_to_observe.add_observer(observer).
   This is the python way, because we just treat the observed thing as an object and call methods on it, in this case add_observer. Calling methods with . is more intuitive than using += which is normally used for arithmetic.
2. I don't think that most users want observing objects to stay alive just by virtue of being observers. I have written a simple implementation of the observer pattern in python to demonstrate how this can be achieved using weak references even in the case that the observed object is a bound method on an instance of an un-hashable class. I emphasize that I would prefer to bring my code into obsub rather than maintain yet another implementation, but I will only spend the time to integrate this if this is a change we agree obsub wants.
3. Ability to observer-ify functions, bound methods, class methods, and static methods would be nice. This is non trivial to accomplish, and I think it is substantially easier if it is allowed to have different decorators for each case. I think this is not a bad thing. Getting everything to work with one decorator is maybe possible but very complicated.
4. It should be possible to specify whether or not the observed object should pass itself as an argument to the observer objects. This could easily be implemented via a parameter in add_observer. Note that this is impossible using the += operator, which is another reason to switch.

Here is my implementation of the observer pattern. You can see how I handle weak reference management in a nice way

https://github.com/DanielSank/observed

[coldfix]

First off: sorry for not answering your post in the mailing list. I had have some quite some projects on my own and was a little too lazy to look into your implementation.

About your comments:

1. I think as well, that += and -= are not a good choice, because they don't make it obvious to the reader, what the action is (they are not only used for arithmetic though, consider concatenation of strings/lists). In my pull-request I chose the names connect/disconnect, but maybe, these are not very much clearer than +=/-= and add_observer could be a better choice of name.
2. I disagree with you here. I had several instances where a library (for example, see here) did not increase the reference count of an observer object, meaning I had to store a reference to it globally in some place. Furthermore, keeping a normal ref is more flexible and straight-forward than keeping a weak-ref, for the following reason: you can pass in a wrapper that stores a weak-ref to the actual object. This means, the normal-ref code handles both cases, whereas the weak-ref code is limited to weak-refs only. See this example:

import weakref

# in lack of a better name:
class WeakObserver(object):

    def __init__(self, observer):
        self._wr = weakref.ref(observer)

    def __call__(self, *args, **kwargs):
        obj = self._wr()
        if obj is not None:
            return obj(*args, **kwargs)

x.add_observer(WeakObserver(obs))

# or, if providing a special function `add_weak_observer` one could
# handle the `ref is None` case as well to automatically disconnect
# the invalid reference.
x.add_weak_observer(obs)

I haven't had much thought about this so far, but it seems to me that you are right insofar that this might be a common need and worth adding to the module, possibly with an extra add_weak_observer() method. I just don't think it should be the default (forbid only) case.

3. The event class in obsub is for observing multiple events (implemented by functions) on objects. If I understand you correctly, you want to have an unbound event (without object), this is exactly what the obsub.signal function does. This one is not in the main module yet, but only in the pull-request I opened a few months ago.

4. I don't know whether the extra switch is really necessary, when in the few cases where the self-argument is needed, you can just connect a lambda that adds the extra argument. On the other hand, this will prevent automatic reference counting and make it necessary to delete the observed object with the garbage collector (depending on your python implementation that might be true either way). But it's worth thinking about the extra switch, if it doesn't hurt the tidiness of the code.

About your repository:

I didn't look at it in detail so far. There are some minor things that I have noticed:

• for now, I'd suggest to keep this as one module (single file) instead of a package, as this makes it easier to include in other code-bases. The example.py file is nice as part of the repository, but doesn't belong to the package, IMO.
• lowerCamelCase is not considered pythonic, instead use lowercase_names_with_underscores for methods and variables, see PEP8
• the code looks a little complicated, I will have to take a deeper look in the next days. In the meantime, you could take a look at my black-magic branch to see if it partially fits your expectations.

[DanielSank]

Thanks for the reply.

1. add_observer and discard_observer seem very descriptive to me ;)

2. This point warrants real discussion.

   I had several instances where a library (for example, see here) did not increase the reference count of an observer object, meaning I had to store a reference to it globally in some place.

   I hadn't really understood why this case would happen. Thank you for pointing this out.

   you can pass in a wrapper that stores a weak-ref to the actual object.

   This is an option, but one must implement the proper weak reference callbacks to clean up when the weakly referenced object dies. I'm not sure how to do that if we pass wrapper objects because the wrapper object has no way to tell the observed object to drop the strong reference to the wrapper. In other words, I can't think of how to write a weak reference callback to do what we want in this case. Maybe there's some trick to pull here that I don't know, but in the end I strongly dislike requiring that observers be wrapped in another class. It will confuse both users and developers. We should be able to do observed_thing.add_observer(observing_thing) without any further monkey business.

   This situation is handled in a very simple way in my implementation, and is probably the most useful thing I have to offer here.

   it seems to me that you are right insofar that this might be a common need and worth adding to the module.

   Yes, this is a very common use case (eg. connecting logic objects to GUI objects), and keeping strong references in the observed code is a memory leak in this case. In fact, this is the reason I made my own implementation instead of using obsub.

3. Regarding observing both functions and bound methods:

   this is exactly what the obsub.signal function does. This one is not in the main module yet, but only in the pull-request I opened a few months ago.

   Ah, I did not know about this. For what it's worth I figured out how to make a decorator that works on both bound methods and functions. That's probably why my implementation looks complicated. I have the same class acting as a descriptor (for the bound method case) and as a callable to which we can attach observers (for the function case and for specific object instances in the bound method case). It's not the best solution but I did it so that there's only one decorator instead of two. I might change that, because having to type two different decorator names seems ok if they're decorating two different things. In fact, this might be better. What do you think?

4. Regarding the switch for the observed object passing itself to observers:

   But it's worth thinking about the extra switch, if it doesn't hurt the tidiness of the code.

   First of all, functionality is more important than super-duper tidy code, right? This is really not hard to add (see my implementation if you're interested), and by having a switch we avoid mind bending things like adding lambdas. The less stuff the user has to know to do the more useful is the library.

Thanks for the feedback on observed. I habitually use camelCase because of Twisted and I'm trying to stop doing that. As for the code being "complicated", this is really useful feedback because it means I need to think carefully about how it's written. Thanks.

for now, I'd suggest to keep this as one module (single file) instead of a package, as this makes it easier to include in other code-bases.

If you have a few minutes, I would deeply appreciate it if you could explain why this is. It's probably more appropriate to post as an issue in my repo though, as we are supposed to be talking about obsub here :P

[coldfix]

I strongly dislike requiring that observers be wrapped in another class. It will confuse both users and developers.

From a user's perspective, I find a wrapper less confusing than weak-refing by default, this is subjective though.

This situation is handled in a very simple way in my implementation, and is probably the most useful thing I have to offer here.

Can't say that I find your implementation simple. All the weak-ref assumptions make it very verbose and hard to read. It is definitely not like canonical python code looks like. I'd say, that this is okay, if it really were necessary, but it's not. Adding a add_weak_observer method to my (hard-ref) code takes a few extra lines (handling disconnection on finalization as well). Adding hard-ref observers to your weak-ref code will be no trivial thing to do.

Yes, this is a very common use case (eg. connecting logic objects to GUI objects), and keeping strong references in the observed code is a memory leak in this case

I'm still convinced weak referencing is not the default thing to do, but a very special occasion. Especially in python, you can never rely on your observer objects being finalized at any given time. Garbage collection is undeterministic and reference counting is an implementation detail of CPython (not pypy for example). So you have to manage the observation times manually either way.

Furthermore, weakrefs are available only for objects with a __dict__. This might not be a very common case, but you can create a callable observer using __slots__.

I might change that, because having to type two different decorator names seems ok if they're decorating two different things.

Yes, I'd certainly advice you to do that, as these are conceptually really different, IMO. I'm a bit KISS/unix minded: every component should do one thing and do it well. I admit, the names signal/event are bad choices (for lack of better ideas) and are pretty undescriptive.

First of all, functionality is more important than super-duper tidy code, right? This is really not hard to add (see my implementation if you're interested)

I wouldn't generalize that too much. In many cases, I'd even say removing functionality is more important than tidy code, because it can make the code conceptually easier.

, and by having a switch we avoid mind bending things like adding lambdas. The less stuff the user has to know to do the more useful is the library.

Lambdas are not mind-bending but part of the language and a well-known technique to add/remove arguments (not saying it is pretty or good practice, though). I'd rather have concerns that an extra parameter is more API the user has to learn, and more importantly: an occasional reader won't understand.

Don't be discouraged by my opposing opinion. I really appreciate your input which made me start to think about some of these topics for the first time as well. And in the end, there are different needs in different cases. So, maybe it's fine, that you have your own repo with your own implementation. I'd be willing to add weak references to my black-magic branch though.

It's probably more appropriate to post as an issue in my repo though

I think, I can give you some more feedback on your implementation later on. Be a bit patient though.

[DanielSank]

Garbage collection is undeterministic and reference counting is an implementation detail of CPython (not pypy for example). So you have to manage the observation times manually either way.

I had not appreciated this. If observers are always removed manually then the use case I described which lead me to consider weak references is already covered. On the other hand, I can imagine people not knowing that ref counting is specific to CPython (I didn't until just now and I've been using python for years) and assuming that their observers will go away when the ref count goes to zero. However, the obsub documentation clearly states that being an observer keeps objects alive, so users have been properly warned and I can probably retract my previous statement that weak referencing is an important addition [1]. It may be worth a simple statement in the documentation explaining this.

[1] Although I maintain that it's really convenient in some cases :)

[coldfix]

I agree with you, that it can come in handy sometimes, and there are applications specifically developed in CPython. But even then, personally, I wouldn't want to rely on reference counting in all but the most simple cases, since it is so easy to add circular references anywhere (oops, I stored an instance method somewhere).