GNOME Bugzilla – Bug 676746
Can't call GdkWindow.raise() due to reserved word
Last modified: 2012-06-25 07:46:09 UTC
'raise' is reserved in Python, meaning that you can't call GdkWindow.raise: >>> w = Gtk.Window() >>> w.realize() >>> w.get_window().raise()
+ Trace 230268
w.get_window().raise()
Workaround: >>> getattr(w.get_window(), 'raise')() PyGTK worked around this by calling the method raise_() More info: https://mail.gnome.org/archives/python-hackers-list/2011-December/msg00011.html
Created attachment 214872 [details] [review] Override GdkWindow.raise() to GdkWindow.raise_() due to reserved word in Python
The patch looks OK to me but it might be better to make pygi detect this case and create the attr with the _ automatically, for this and other reserved keywords.
Yes, Tomeu was suggesting the same and pointed to a possible place to do that: http://git.gnome.org/browse/pygobject/tree/gi/types.py#n87
Created attachment 215051 [details] [review] initial patch attempt This is an initial attempt to implement this. I don't know if def _setup_fields(cls): and def _setup_constants(cls): also need this. Test are missing, maybe someone can help me with them.
Comment on attachment 214872 [details] [review] Override GdkWindow.raise() to GdkWindow.raise_() due to reserved word in Python I think we should rather go with the generic solution. Thanks!
For a test case, I'd rather resort to using identifiers in GLib and GObject. GObject does not have any which are Python keywords, but GLib actually has a few: Variant.print() Timer.continue() BookmarkFile.add_application() has an "exec" argument Thread.yield() My search also brought up IOCondition.in, but as these are presented as upper-case in Python it doesn't count. (In reply to comment #4) > I don't know if def _setup_fields(cls): and def _setup_constants(cls): also > need this. Most likely not constants as they are in upper case (see above), but probably for field names; we'll need to add a field name to gobject-introspection's GIMarshallingTests for this.
Unfortunately all these existing examples in GLib are not GObjects, but structs and fundamentals, so the patch in MetaClassHelper is not sufficient. E. g. it does not help for v = GLib.Variant('i', 1) v.print_(False) How about we tie this into _wrap_g_base_info_get_name()? This is as close to GI as we can get, and the first step into the Python realm, where we actually can check for Python keywords.
Created attachment 215408 [details] [review] Escape identifiers which are Python keywords How about this patch? Peer review appreciated; in particular, I'm looking for opinions about hardcoding the keyword list for every major release or whether it's better to do the (more correct, but much more expensive) keyword.iskeyword() call to the Python module.
Summary of discussion with Dieter: - We could consider generating the keyword list at build time (into a .h file which we #include), and then just add the special cases. - We only want this list to grow monotonously with newer major versions, to avoid breaking existing API (e. g. if they ever drop the "print" keyword, we still want existing my_variant.print_() calls to continue to work. - This patch will #error out when we build against a Python 4 or later, and shows the command how to generate the keyword list (at least for Python 2 and 3), so it's not a hidden trap. - This is not the most elegant approach, but very robust.
Review of attachment 215408 [details] [review]: the patch looks good to me and I agree that this approach is robust
Comment on attachment 215408 [details] [review] Escape identifiers which are Python keywords Thanks for the review. Pushed with a proper changelog and some minor tweaks in the new comment.