GNOME Bugzilla – Bug 727810
Support setattr pass-through based on union discrimination for Gdk.Event
Last modified: 2014-08-23 04:57:47 UTC
Created attachment 273774 [details] Different approaches on emitting Event/EventKey I have no idea how to emit a correct event in analogy to PyGTK methods. I attach an example code with comments to give an idea how different approaches have failed. The direct reason for this bug report is an error message in one variant: TypeError: cannot set a structure which has no well-defined ownership transfer rules
################################################## def send_key_event(widget, **kwargs): """Returns True if the event was handled""" ev = Gdk.Event() ev.type = Gdk.EventType.KEY_PRESS ev.any.window = win.get_window() for key, value in kwargs.items(): setattr(ev.key, key, value) return widget.event(ev) print(send_key_event(entry, keyval=ord("A"))) ##################################################
Thank you. I have missed the possibility of setting those keys directly.
The reason why setting fields of the Gdk.Event enum is weird is that it does some magic getting the enum field members depending on the type but doesn't do the same for setting values. E.g. getattr(ev, "window") works while setattr(ev, "window", ..) doesn't. Also Gdk.Event.__new__ ignores all arguments. "Gdk.Event(Gdk.EventType.KEY_PRESS)" does not set the type, only "Gdk.Event.new(Gdk.EventType.KEY_PRESS)" does. I propose the following changes: 1) The __getattr__ magic of Gdk.Event should be mirrored to __setattr__. 2) Passing arguments to union constructors should result in a deprecation warning since they get just ignored atm. This is handled in _boxed_new()/pygi-boxed.c as far as I can see.
(In reply to comment #3) > ... > I propose the following changes: > > 1) The __getattr__ magic of Gdk.Event should be mirrored to __setattr__. > > 2) Passing arguments to union constructors should result in a deprecation > warning since they get just ignored atm. This is handled in > _boxed_new()/pygi-boxed.c as far as I can see. Sounds good to me. Also note there is a union discriminator API in GI but I don't think it is actually implemented on the scanner/typelib side of things: https://developer.gnome.org/gi/1.40/gi-GIUnionInfo.html#g-union-info-get-discriminator-type
Additionally it would be nice to implement __repr__ to return something a bit more descriptive like: Gdk.Event.new(Gdk.EventType.KEY_PRESS)
Created attachment 274287 [details] [review] Gdk.Event: Include GdkEventType in __repr__ Adds event type to the repr, but not in the format you proposed, since repr() of GdkEventType would also have to be changed. So I kept it simple and consistent for now.
Created attachment 274288 [details] [review] Gdk.Event: Override __setattr__ to set fields base on the Turned out there was a test for this, but it was buggy and only set Gdk.Event.__dict__ side note: the UNION_MEMBERS list is out of date, there are new event types in gdk..
Created attachment 274289 [details] [review] Set deprecation warning if arguments get passed to gi.Boxed.__init__() This checks on __init__, thus all overrides using __new__ need to filter out __init__ args. Not 100% sure if this is the best approach, feedback welcome.
Review of attachment 274287 [details] [review]: Looks good.
Review of attachment 274288 [details] [review]: LGTM.
Review of attachment 274289 [details] [review]: This patch is related to bug 705810: https://git.gnome.org/browse/pygobject/commit/?id=2f2069c9efcd8 Which removed an args/kwargs check from the GIBoxed.__new__ (_boxed_new). Unfortunately the reason for that removal wasn't clearly noted in the commit log (and should have been put in a separate patch). But it allowed removal of various __new__ implementations for things like Gdk.Event and Gdk.Color. I think needing to implement __new__ and __init__ for those boxed overrides annoyed me, but the removal caused this bug to occur. I like your approach of putting the error in _boxed_init rather than in _boxed_new. Similarly we might be able to get away with a RuntimeWarning or even an exception instead of PyGIDeprecationWarning since Boxed creation with extra args used to be an exception pre-3.12. The difference between putting the error in _boxed_new vs _boxed_init is with _boxed_new, any overridden __init__ which accepts arguments also needs to override __new__ to ignore args. With _boxed_init giving the error the reverse is true when overriding __new__... it requires an __init__ override which ignores args, but only having an __init__ override does not require a __new__ override. I think the error in _boxed_init (what you've done) seems cleaner as only overriding __init__ is arguably a more common Python idiom. So this patch seems fine with the addition of a possibly harder error and Pango.Layout not needing the override. ::: gi/overrides/Pango.py @@ +48,3 @@ + def __init__(self, *args, **kwargs): + return super(Layout, self).__init__() Pango.Layout is a GObject so this shouldn't be needed.
Created attachment 274308 [details] [review] Raise TypeError if arguments get passed to Boxed.__init__
Attachment 274287 [details] pushed as 78a0508 - Gdk.Event: Include GdkEventType in __repr__
Note I changed this back to use the warning code you added from the patch in comment #8 with the difference that it is a TypeError not a DeprecationWarning. The exception caused some fallout in at least Gramps. https://git.gnome.org/browse/pygobject/commit/?id=27a1467