GNOME Bugzilla – Bug 692918
Replace static signal related methods with introspected versions
Last modified: 2014-08-18 10:35:02 UTC
The following static methods should be easy pickings with either bug 633999 being fixed or adding a __gpointer__ property to the static GObject. The latter idea seems like it will also allow us to use the methods for other fundamental object types. pygobject_disconnect pygobject_handler_is_connected pygobject_handler_block pygobject_handler_unblock pygobject_stop_emission And with a bit of work we should be able to remove the following: pygobject_connect pygobject_connect_after pygobject_connect_object pygobject_connect_object_after pygobject_emit
Created attachment 234887 [details] [review] Add __gpointer__ property to GObject static binding Add access to the underlying C GObject pointer by wrapping it in a PyCapsule/PyCPointer and exposing it as __gpointer__. Add special case marshaling for gi parameters annotated as gpointer to accept a PyCapsule and extract the underlying pointer as the arg. This allows usage of methods like GObject.signal_handler_disconnect which we can start replacing the static bindings with. Having a hard time coming up with isolated tests, but it will definitely be tested by replacing static signal methods in an additional patch.
Created attachment 234892 [details] [review] Add tests for signal stop_emission, disconnect, and handler_is_connected Add tests for methods which will be moving from static bindings to gi by using the new __gpointer__ attribute of GObject.
Created attachment 234893 [details] [review] Move various signal methods from static bindings to gi and python Move disconnect, handler_is_connected, handler_block, handler_unblock, and stop_emission from static to gi python overrides. The only contentious part of this might be that stop_emission no longer raises a TypeError when an invalid signal name is passed. The reason I don't think this is needed is because the exception is raised within a closure which gets flattened into a printed message anyhow. So we might as well save a bit of code and rely on the the default GLib warning instead.
Comment on attachment 234893 [details] [review] Move various signal methods from static bindings to gi and python Thanks for those, Simon! To clarify, the main difference in behaviour here is that stop_emission() would now merely raise a warning instead of throwing a TypeError for invalid signals, right? But that's ok IMHO, as GLib does the same and does not return a "success" value. I have one request, though: Can you please make the "backwards compatible" names raise a PyGIDeprecationWarning, so that we can eventually remove them? E. g. "stop_emission = stop_emission_by_name" hides access to g_signal_stop_emission(), at some point we want to make this accessible to Python.
Comment on attachment 234887 [details] [review] Add __gpointer__ property to GObject static binding This seems fine to have even without the other patches.
(In reply to comment #4) > Thanks for those, Simon! To clarify, the main difference in behaviour here is > that stop_emission() would now merely raise a warning instead of throwing a > TypeError for invalid signals, right? But that's ok IMHO, as GLib does the same > and does not return a "success" value. Yes, furthermore if the exception is not explicitly caught by a try/except within the signal handler, it will just get flattened to a logged warning anyhow when the closure exists. > I have one request, though: Can you please make the "backwards compatible" > names raise a PyGIDeprecationWarning, so that we can eventually remove them? E. > g. "stop_emission = stop_emission_by_name" hides access to > g_signal_stop_emission(), at some point we want to make this accessible to > Python. The "backwards compatible" comment was a bit of a misnomer. Because disconnect is a real method on GObject, but the static python bindings only ever accepted a signal_id, whereas the C method takes a GCallback with variable arguments. So this basically continues to make a real method work, just a bit differently than the C API. Some of these are also just python only convenience aliases mapped from the global "g_signal_*" functions to something usable directly on a GObject, which makes sense at least for now. Perhaps at this point we should just change the comment to "aliases". And at a later point, take a pass to consolidate the signal related methods into a nicer API which attempts more parity with the C API. So for instance, deprecating "disconnect" might cause a lot of disruption and it is unclear at this point if that is the right thing to do. What do you think?
Comment on attachment 234887 [details] [review] Add __gpointer__ property to GObject static binding Attachment 234887 [details] pushed as df18f9c - Add __gpointer__ property to GObject static binding
Comment on attachment 234892 [details] [review] Add tests for signal stop_emission, disconnect, and handler_is_connected Attachment 234892 [details] pushed as a53a917 - Add tests for signal stop_emission, disconnect, and handler_is_connected
(In reply to comment #6) > The "backwards compatible" comment was a bit of a misnomer. Because disconnect > is a real method on GObject, but the static python bindings only ever accepted > a signal_id, whereas the C method takes a GCallback with variable arguments. So > this basically continues to make a real method work, just a bit differently > than the C API. I'm fine with disconnect(), I'm mostly concerned about hiding g_signal_stop_emission() forever because of the backwards compat aliasing it to g_signal_stop_emission_by_name(). > Some of these are also just python only convenience aliases mapped from the > global "g_signal_*" functions to something usable directly on a GObject, which > makes sense at least for now. Oh, right! I thought these ended up as methods in the GIR, but they are indeed module functions. So they aren't hidden indeed. > Perhaps at this point we should just change the comment to "aliases". And at a > later point, take a pass to consolidate the signal related methods into a nicer > API which attempts more parity with the C API. So for instance, deprecating > "disconnect" might cause a lot of disruption and it is unclear at this point if > that is the right thing to do. > What do you think? For the new methods like disconnect(), we can certainly keep them forever. But having three different names for the same one seems unnecessary in the long run, so I'd just pick the one matching the C API as "the" method, and provide deprecated aliases for some cycles.
Created attachment 235052 [details] [review] Move various signal methods from static bindings to gi and python Added deprecation warning for stop_emission and emit_stop_by_name.
Comment on attachment 235052 [details] [review] Move various signal methods from static bindings to gi and python Thanks for the update! This looks great now.
Comment on attachment 235052 [details] [review] Move various signal methods from static bindings to gi and python Attachment 235052 [details] pushed as b31d8a9 - Move various signal methods from static bindings to gi and python
Leaving open as the following are still candidates for static removal: pygobject_connect pygobject_connect_after pygobject_connect_object pygobject_connect_object_after pygobject_emit
The following GObject global functions are available for fixing the remaing static bindings: signal_add_emission_hook signal_connect_closure signal_chain_from_overridden signal_emitv signal_override_class_closure signal_set_va_marshaller signal_type_cclosure_new However, more work needs to go into the marshaling code to make them work better.
Additional notes: I've verified signal_connect_closure works well out of the box and can almost directly replace GObject.Object.connect and connect_after. connect_object and connect_object_after can be simulated in pure python by wrapping the callbacks and swizzling the args. Two issues unfold after this: 1. This breaks Object.handler_block_by_func and Object.handler_unblock_by_func because Object.connect tracks connected Python closures and uses that tracking for these methods. I think this can be fixed by instead keeping a global mapping of Python closure to GClosures used when ever a GClosure is created. This will also allow re-use of closures of the same Python function. 2. Marshaling no longer makes use of GI argument annotations for signals. This can be fixed by allowing GClosure creation to accept an option GICallableInfo hint. This can be used for closure creation during signal related connect functions.
I think this work has been completed as much as it can.