This is perhaps similar to bug 332782.

Created attachment 209808 [details] [review]
Addition of Signal class for decorating and binding custom signals.

The Signal class allows for easy creation of signals and removes the
need for __gsignals__ in client code. The Signal class can also be
used as a decorator for wrapping up the custom closure. It provides
a "BoundSignal" when accessed on an instance allowing for
connections without specifying a signal name string.

Example:
class Thing(GObject.GObject):
    value = 0
    @GObject.Signal
    def pushed(self):
        self.value += 1
    @GObject.Signal(flags=GObject.SignalFlags.RUN_LAST)
    def pulled(self):
        self.value -= 1
    stomped = GObject.Signal('stomped', argTypes=(int,))

def onPushed(obj):
    print obj
thing = Thing()
thing.pushed.connect(onPushed)
thing.pushed.emit()

A couple extra comments on the patch:

* I just realized parameters should be named with lowercase and underscore style. Is this correct? (argTypes should be arg_types)
* The class name itself was named with title case (Signal) to match the style of other classes in GObject. But differs from the "property" class which I propose we rename: https://bugzilla.gnome.org/show_bug.cgi?id=672168
* The class is derived from str so that it can be passed into GObject.connect without problems. I would prefer deriving from object but then GObject.connect would need an override to run str() on the input signal name.

Created attachment 209893 [details] [review]
2. Addition of Signal class for decorating and binding custom signals.

* Renamed argument and instance vars to lowercase with underscore.
* Fixed potential signal name / class variable naming discrepancy.

I haven't done a detailled code review yet, but I generally like the idea.

However, as this is quite intrusive, I propose we keep this for post-3.2 (i. e. after GNOME 3.4), as GNOME 3.4 is already deep in feature/API freeze, and next Monday is hard code freeze already.

Thanks for working on this!

P.S. Just caught my eye: onPulled -> on_pulled in the example doc string

Sounds fine with me. It does need more review and I would like to look into avoiding deriving from str and making connect more robust in that it calls "str" on input signal parameters.

Created attachment 210171 [details] [review]
Addition of Signal class for adding and connecting custom signals

* Fixed unnamed signals (uses class variable name like Property).
* Added more unittests and examples.

Review of attachment 210171 [details] [review]:

Thanks for starting to do this work, this will be pretty nice.

A couple of design questions follows.

::: examples/properties.py
@@ +3,2 @@
 class MyObject(GObject.GObject):
+    _intdata = 0

This should be an instance variable.

::: examples/signal.py
@@ +7,3 @@
         print "C: class closure for `my_signal' called with argument", arg
 
+    @GObject.Signal(arg_types=(GObject.TYPE_INT,))

I think this should spell; 

@GObject.Signal(int)

Eg, varargs should be the parameter types.

::: gi/_gobject/Makefile.am
@@ +31,3 @@
 	constants.py \
+	propertyhelper.py \
+	signal.py

signalhelper.py to follow the style?

::: gi/_gobject/__init__.py
@@ +109,3 @@
         cls.do_set_property = obj_set_property
 
+    def _install_signals(cls):

Could parts of this be moved to a Signal classmethod to keep most of the implementation in the same module?

@@ +123,3 @@
+                    setattr(cls, name, signal)
+                if signalName in gsignals:
+                # Fixup a signal which is unnamed by using the class variable name.

This exception message could be improved.

::: gi/_gobject/signal.py
@@ +35,3 @@
+        def pulled(self):
+            self.value -= 1
+        stomped = GObject.Signal('stomped', arg_types=(int,))

I'm not sure if the non-decorated case should be supported.
I think it's perfectly fine to require a default method, eg, require a function in constructor.

@@ +43,3 @@
+    obj.pulled.emit()
+    """
+    class BoundSignal(str):

This is pretty weird, can't we just subclass from object?

Review of attachment 210171 [details] [review]:

Thanks for the review. I will submit another patch upon the details being worked out.

::: examples/properties.py
@@ +3,2 @@
 class MyObject(GObject.GObject):
+    _intdata = 0

I did this for brevity, upon setting the variable using 'self._intdata = value' it will become an instance variable. It can be changed but I don't think it really matters either way.

::: examples/signal.py
@@ +7,3 @@
         print "C: class closure for `my_signal' called with argument", arg
 
+    @GObject.Signal(arg_types=(GObject.TYPE_INT,))

I don't get how this could work. You are suggesting *args be used in place of the explicit "arg_types"? If this were the case, you would only have the ability to initialize a Signal object with callback arg types as any other key arg would immediately be clobbered by what was intended to be used as "arg_types". For instance if the __init__ signature looked like this:
  def __init__(self, flags=0, *args)
Then calling Signal(int) would apply int as the flags parameter and *args would be empty.

Python3 gives us some extra foo that we can play with in regards to inspectable parameter annotations on the closure. For instance, we could use syntax like this when creating a signal:

@Signal
def my_signal(self, amount:float, count:int) -> int:
    return 2

The Signal decorator can pull that meta information out of the function and use it for creating the GObject signal. But this is only supported in python3 so it could come at a later time.

::: gi/_gobject/Makefile.am
@@ +31,3 @@
 	constants.py \
+	propertyhelper.py \
+	signal.py

I dislike things named with either helper or util as these words don't add any more meaning to what is already there. I'd vote for renaming propertyhelper.py to property.py instead, but if you guys aren't ok with that then keeping them consistently named with helper is better then having them different.

::: gi/_gobject/__init__.py
@@ +109,3 @@
         cls.do_set_property = obj_set_property
 
+    def _install_signals(cls):

I think so, I would revise and see how much of it can be moved.

@@ +123,3 @@
+                    setattr(cls, name, signal)
+                if signalName in gsignals:
+                # Fixup a signal which is unnamed by using the class variable name.

Sure thing.

::: gi/_gobject/signal.py
@@ +35,3 @@
+        def pulled(self):
+            self.value -= 1
+        stomped = GObject.Signal('stomped', arg_types=(int,))

The non-decorated usage is actually my primary use case for these. As I don't usually use a closure along with the signal.

@@ +43,3 @@
+    obj.pulled.emit()
+    """
+    class BoundSignal(str):

I agree it seems a bit odd. The reason is for supporting usage of: 
 obj.connect(obj.signal, callback)

So the Signal instance at the class or instance level can be used as an arg to GObject.connect. The other route here would be to change all of the GObject.connect* methods to use 'str' on it's input but this would be a lot of work in comparison with simply deriving from str. But I would be fine with doing that work as well.

Review of attachment 210171 [details] [review]:

Thanks for the review. I will submit another patch upon the details being worked out.

::: examples/properties.py
@@ +3,2 @@
 class MyObject(GObject.GObject):
+    _intdata = 0

I did this for brevity, upon setting the variable using 'self._intdata = value' it will become an instance variable. It can be changed but I don't think it really matters either way.

::: examples/signal.py
@@ +7,3 @@
         print "C: class closure for `my_signal' called with argument", arg
 
+    @GObject.Signal(arg_types=(GObject.TYPE_INT,))

I don't get how this could work. You are suggesting *args be used in place of the explicit "arg_types"? If this were the case, you would only have the ability to initialize a Signal object with callback arg types as any other key arg would immediately be clobbered by what was intended to be used as "arg_types". For instance if the __init__ signature looked like this:
  def __init__(self, flags=0, *args)
Then calling Signal(int) would apply int as the flags parameter and *args would be empty.

Python3 gives us some extra foo that we can play with in regards to inspectable parameter annotations on the closure. For instance, we could use syntax like this when creating a signal:

@Signal
def my_signal(self, amount:float, count:int) -> int:
    return 2

The Signal decorator can pull that meta information out of the function and use it for creating the GObject signal. But this is only supported in python3 so it could come at a later time.

::: gi/_gobject/Makefile.am
@@ +31,3 @@
 	constants.py \
+	propertyhelper.py \
+	signal.py

I dislike things named with either helper or util as these words don't add any more meaning to what is already there. I'd vote for renaming propertyhelper.py to property.py instead, but if you guys aren't ok with that then keeping them consistently named with helper is better then having them different.

::: gi/_gobject/__init__.py
@@ +109,3 @@
         cls.do_set_property = obj_set_property
 
+    def _install_signals(cls):

I think so, I would revise and see how much of it can be moved.

@@ +123,3 @@
+                    setattr(cls, name, signal)
+                if signalName in gsignals:
+                # Fixup a signal which is unnamed by using the class variable name.

Sure thing.

::: gi/_gobject/signal.py
@@ +35,3 @@
+        def pulled(self):
+            self.value -= 1
+        stomped = GObject.Signal('stomped', arg_types=(int,))

The non-decorated usage is actually my primary use case for these. As I don't usually use a closure along with the signal.

@@ +43,3 @@
+    obj.pulled.emit()
+    """
+    class BoundSignal(str):

I agree it seems a bit odd. The reason is for supporting usage of: 
 obj.connect(obj.signal, callback)

So the Signal instance at the class or instance level can be used as an arg to GObject.connect. The other route here would be to change all of the GObject.connect* methods to use 'str' on it's input but this would be a lot of work in comparison with simply deriving from str. But I would be fine with doing that work as well.

Review of attachment 210171 [details] [review]:

Thanks for the review. I will submit another patch upon the details being worked out.

::: examples/properties.py
@@ +3,2 @@
 class MyObject(GObject.GObject):
+    _intdata = 0

I did this for brevity, upon setting the variable using 'self._intdata = value' it will become an instance variable. It can be changed but I don't think it really matters either way.

::: examples/signal.py
@@ +7,3 @@
         print "C: class closure for `my_signal' called with argument", arg
 
+    @GObject.Signal(arg_types=(GObject.TYPE_INT,))

I don't get how this could work. You are suggesting *args be used in place of the explicit "arg_types"? If this were the case, you would only have the ability to initialize a Signal object with callback arg types as any other key arg would immediately be clobbered by what was intended to be used as "arg_types". For instance if the __init__ signature looked like this:
  def __init__(self, flags=0, *args)
Then calling Signal(int) would apply int as the flags parameter and *args would be empty.

Python3 gives us some extra foo that we can play with in regards to inspectable parameter annotations on the closure. For instance, we could use syntax like this when creating a signal:

@Signal
def my_signal(self, amount:float, count:int) -> int:
    return 2

The Signal decorator can pull that meta information out of the function and use it for creating the GObject signal. But this is only supported in python3 so it could come at a later time.

::: gi/_gobject/Makefile.am
@@ +31,3 @@
 	constants.py \
+	propertyhelper.py \
+	signal.py

I dislike things named with either helper or util as these words don't add any more meaning to what is already there. I'd vote for renaming propertyhelper.py to property.py instead, but if you guys aren't ok with that then keeping them consistently named with helper is better then having them different.

::: gi/_gobject/__init__.py
@@ +109,3 @@
         cls.do_set_property = obj_set_property
 
+    def _install_signals(cls):

I think so, I would revise and see how much of it can be moved.

@@ +123,3 @@
+                    setattr(cls, name, signal)
+                if signalName in gsignals:
+                # Fixup a signal which is unnamed by using the class variable name.

Sure thing.

::: gi/_gobject/signal.py
@@ +35,3 @@
+        def pulled(self):
+            self.value -= 1
+        stomped = GObject.Signal('stomped', arg_types=(int,))

The non-decorated usage is actually my primary use case for these. As I don't usually use a closure along with the signal.

@@ +43,3 @@
+    obj.pulled.emit()
+    """
+    class BoundSignal(str):

I agree it seems a bit odd. The reason is for supporting usage of: 
 obj.connect(obj.signal, callback)

So the Signal instance at the class or instance level can be used as an arg to GObject.connect. The other route here would be to change all of the GObject.connect* methods to use 'str' on it's input but this would be a lot of work in comparison with simply deriving from str. But I would be fine with doing that work as well.

Review of attachment 210171 [details] [review]:

Thanks for the review. I will submit another patch upon the details being worked out.

::: examples/properties.py
@@ +3,2 @@
 class MyObject(GObject.GObject):
+    _intdata = 0

I did this for brevity, upon setting the variable using 'self._intdata = value' it will become an instance variable. It can be changed but I don't think it really matters either way.

::: examples/signal.py
@@ +7,3 @@
         print "C: class closure for `my_signal' called with argument", arg
 
+    @GObject.Signal(arg_types=(GObject.TYPE_INT,))

I don't get how this could work. You are suggesting *args be used in place of the explicit "arg_types"? If this were the case, you would only have the ability to initialize a Signal object with callback arg types as any other key arg would immediately be clobbered by what was intended to be used as "arg_types". For instance if the __init__ signature looked like this:
  def __init__(self, flags=0, *args)
Then calling Signal(int) would apply int as the flags parameter and *args would be empty.

Python3 gives us some extra foo that we can play with in regards to inspectable parameter annotations on the closure. For instance, we could use syntax like this when creating a signal:

@Signal
def my_signal(self, amount:float, count:int) -> int:
    return 2

The Signal decorator can pull that meta information out of the function and use it for creating the GObject signal. But this is only supported in python3 so it could come at a later time.

::: gi/_gobject/Makefile.am
@@ +31,3 @@
 	constants.py \
+	propertyhelper.py \
+	signal.py

I dislike things named with either helper or util as these words don't add any more meaning to what is already there. I'd vote for renaming propertyhelper.py to property.py instead, but if you guys aren't ok with that then keeping them consistently named with helper is better then having them different.

::: gi/_gobject/__init__.py
@@ +109,3 @@
         cls.do_set_property = obj_set_property
 
+    def _install_signals(cls):

I think so, I would revise and see how much of it can be moved.

@@ +123,3 @@
+                    setattr(cls, name, signal)
+                if signalName in gsignals:
+                # Fixup a signal which is unnamed by using the class variable name.

Sure thing.

::: gi/_gobject/signal.py
@@ +35,3 @@
+        def pulled(self):
+            self.value -= 1
+        stomped = GObject.Signal('stomped', arg_types=(int,))

The non-decorated usage is actually my primary use case for these. As I don't usually use a closure along with the signal.

@@ +43,3 @@
+    obj.pulled.emit()
+    """
+    class BoundSignal(str):

I agree it seems a bit odd. The reason is for supporting usage of: 
 obj.connect(obj.signal, callback)

So the Signal instance at the class or instance level can be used as an arg to GObject.connect. The other route here would be to change all of the GObject.connect* methods to use 'str' on it's input but this would be a lot of work in comparison with simply deriving from str. But I would be fine with doing that work as well.

Review of attachment 210171 [details] [review]:

Thanks for the review. I will submit another patch upon the details being worked out.

::: examples/properties.py
@@ +3,2 @@
 class MyObject(GObject.GObject):
+    _intdata = 0

I did this for brevity, upon setting the variable using 'self._intdata = value' it will become an instance variable. It can be changed but I don't think it really matters either way.

::: examples/signal.py
@@ +7,3 @@
         print "C: class closure for `my_signal' called with argument", arg
 
+    @GObject.Signal(arg_types=(GObject.TYPE_INT,))

I don't get how this could work. You are suggesting *args be used in place of the explicit "arg_types"? If this were the case, you would only have the ability to initialize a Signal object with callback arg types as any other key arg would immediately be clobbered by what was intended to be used as "arg_types". For instance if the __init__ signature looked like this:
  def __init__(self, flags=0, *args)
Then calling Signal(int) would apply int as the flags parameter and *args would be empty.

Python3 gives us some extra foo that we can play with in regards to inspectable parameter annotations on the closure. For instance, we could use syntax like this when creating a signal:

@Signal
def my_signal(self, amount:float, count:int) -> int:
    return 2

The Signal decorator can pull that meta information out of the function and use it for creating the GObject signal. But this is only supported in python3 so it could come at a later time.

::: gi/_gobject/Makefile.am
@@ +31,3 @@
 	constants.py \
+	propertyhelper.py \
+	signal.py

I dislike things named with either helper or util as these words don't add any more meaning to what is already there. I'd vote for renaming propertyhelper.py to property.py instead, but if you guys aren't ok with that then keeping them consistently named with helper is better then having them different.

::: gi/_gobject/__init__.py
@@ +109,3 @@
         cls.do_set_property = obj_set_property
 
+    def _install_signals(cls):

I think so, I would revise and see how much of it can be moved.

@@ +123,3 @@
+                    setattr(cls, name, signal)
+                if signalName in gsignals:
+                # Fixup a signal which is unnamed by using the class variable name.

Sure thing.

::: gi/_gobject/signal.py
@@ +35,3 @@
+        def pulled(self):
+            self.value -= 1
+        stomped = GObject.Signal('stomped', arg_types=(int,))

The non-decorated usage is actually my primary use case for these. As I don't usually use a closure along with the signal.

@@ +43,3 @@
+    obj.pulled.emit()
+    """
+    class BoundSignal(str):

I agree it seems a bit odd. The reason is for supporting usage of: 
 obj.connect(obj.signal, callback)

So the Signal instance at the class or instance level can be used as an arg to GObject.connect. The other route here would be to change all of the GObject.connect* methods to use 'str' on it's input but this would be a lot of work in comparison with simply deriving from str. But I would be fine with doing that work as well.

Sorry for the spam, that publish button seems to be problematic especially when you aren't logged in!

(In reply to comment #13)
> Thanks for the review. I will submit another patch upon the details being
> worked out.

Thanks. Setting the current one to "needs-work" to reflect the status and clean the patch review queue.

> @@ +31,3 @@
>      constants.py \
> +    propertyhelper.py \
> +    signal.py
> 
> I dislike things named with either helper or util as these words don't add any
> more meaning to what is already there. I'd vote for renaming propertyhelper.py
> to property.py instead

+1 from me. This is mostly bikeshedding of course. If there is no objection, I'll rename it.

Created attachment 218214 [details] [review]
Patch updates

This latest patch includes the following fixes:
- Renamed signal.py to signalhelper.py for consistency.
- Broke override signal support out into a simplified sub-class named "SignalOverride" for use as a decorator.
- Verified python support from 2.7 to 3.2
- Updated documentation and unittests
- Moved code for managing signal installation from gi/_gobject/__init__.py into gi/_gobject/signalhelper.py
- Fixed problem in test_signal.py where "float" and "double" were being clobbered with GObject.TYPE_FLOAT, renamed usage to gfloat and gdouble respectively.
- Added support for inspecting a signals closure using python3 annotations to denote arg and return types.

Thanks for the update! This looks quite nice now.

We are past feature and API freeze now, so I'm targetting this at GNOME 3.8 so that we can commit it early in the next cycle. Please feel free to ask the release team for an exception, of course!

Thanks Martin,
I made a request to the release-team to include this. 

The patch has been tested against the latest source with Python 2.6, 2.7, 3.1, 3.2, and 3.3 and it's good.

Comment on attachment 218214 [details] [review]
Patch updates

This line should be removed for committing:

458 +            print(args, kargs)

Otherwise this looks fine to me, thanks! I think you already got one +1 from the release team, can you please let me know when you got the second one?

From my POV I'm fine with adding that now, as it does not change existing API and the new features are covered by tests.

Created attachment 222516 [details] [review]
Removed debug print

Same patch with single debugging print removed.

Only a single +1 from the release team so far. I will update if that changes. Thanks.

Just got another +1 from Javier Jardón. So this patch should be good to go for the upcoming release!

Great! I pushed this with a PEP-8 whitespace fix and some changelog improvement.

Thanks a lot!