After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 686765 - Replace static GLib constants with those available in GI
Replace static GLib constants with those available in GI
Status: RESOLVED FIXED
Product: pygobject
Classification: Bindings
Component: introspection
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Martin Pitt
Python bindings maintainers
Depends on:
Blocks: 685373 687488
 
 
Reported: 2012-10-24 07:34 UTC by Simon Feltman
Modified: 2012-11-05 10:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
WIP: drop OPTION_ constants (9.11 KB, patch)
2012-10-24 13:30 UTC, Martin Pitt
needs-work Details | Review
Replace static OPTION_* constants with GI (9.94 KB, patch)
2012-11-05 10:08 UTC, Martin Pitt
accepted-commit_now Details | Review
Replace static OPTION_* constants with GI (10.14 KB, patch)
2012-11-05 10:21 UTC, Martin Pitt
committed Details | Review

Description Simon Feltman 2012-10-24 07:34:31 UTC
It looks like most of the constants can be removed from the static bindings and replaced with what is available in GI. They should also be deprecated eventually in favor of using the Enum and Flags class variables directly.

IO_FLAGS_* = GLib.IOFlags.*
IO_ERR, IO_HUP, IO_IN, IO_NVAL, IO_OUT, IO_PRI = GLib.IOCondition.*
IO_STATUS_* = GLib.IOStatus.*
OPTION_ERROR_* = GLib.OptionError.*
OPTION_FLAG_* = GLib.OptionFlags.*
OPTION_REMAINING = GLib.OPTION_REMAINING
SPAWN_* = GLib.SpawnFlags.*

Because enums and flags coming from GI are based on ints, I think these can be passed into existing static methods without issue.
Comment 1 Martin Pitt 2012-10-24 07:41:28 UTC
Indeed, they should work just fine. I was going to write tests for IOChannel (as we currently do not have any for those) in preparation for GI-ifying those. These will confirm that the GI constants for IO_* will work with the static bindings.
Comment 2 Martin Pitt 2012-10-24 12:01:31 UTC
http://git.gnome.org/browse/pygobject/commit/?id=0137a7af7 drops all static IO_* constants, as they have test coverage.
Comment 3 Martin Pitt 2012-10-24 12:13:39 UTC
http://git.gnome.org/browse/pygobject/commit/?id=6a586af41b drops the static SPAWN_* constants.
Comment 4 Martin Pitt 2012-10-24 12:32:21 UTC
As for the OptionParser stuff, I'm not even sure whether anyone uses that. Google code search reveals nothing but pygobject itself:

http://code.google.com/codesearch#search/&q=GLib.Option%20lang:^python$
http://code.google.com/codesearch#search/&q=GLib.OPTION_

We don't even export most of the API into GLib, as even our test case says:

# FIXME: we need a way to import the options module from a public module
from gi._glib.option import OptionParser, OptionGroup, OptionValueError, \
    make_option, BadOptionError

and http://code.google.com/codesearch#search/&q=gi._glib.option is empty. I guess in Python it is a lot more comfortable to use argparse (or optparse)?

The public API for this is GLib.option (gi/_glib/option.py), so I don't think we need to bother about exporting the old constant names any longer?
Comment 5 Martin Pitt 2012-10-24 13:30:54 UTC
Created attachment 227148 [details] [review]
WIP: drop OPTION_ constants

For the OptionParse stuff this is actually a bit more complicated. We import gi/_glib/option.py into GLib (and GObject) one way or the other (right now through gi/_gi/__init__.py), so we cannot use gi.repository.GLib in gi/_glib/option.py as this would be a circular import.

I guess we'd need to put the whole "option" stuff into the GLib overrides instead.

Attaching my current WIP patch which demonstrates the problem.
Comment 6 Martin Pitt 2012-10-24 13:32:37 UTC
Simon told in IRC he wanted to work on reorganizing the helpers (signalhelper, propertyhelper, option) to avoid this kind of circular dependency, so I'm leaving this for now.
Comment 7 Martin Pitt 2012-11-05 10:08:57 UTC
Created attachment 228086 [details] [review]
Replace static OPTION_* constants with GI

This updated patch now uses get_introspection_module('GLib') to avoid the circular dependency on the overrides, and was rebased against current master.
Comment 8 Simon Feltman 2012-11-05 10:19:31 UTC
Review of attachment 228086 [details] [review]:

Looks good to me!
Comment 9 Martin Pitt 2012-11-05 10:21:49 UTC
Created attachment 228088 [details] [review]
Replace static OPTION_* constants with GI

This puts back full backwards compatibility, just in case.
Comment 10 Martin Pitt 2012-11-05 10:24:28 UTC
Comment on attachment 228088 [details] [review]
Replace static OPTION_* constants with GI

Pushed.
Comment 11 Martin Pitt 2012-11-05 10:25:26 UTC
There are no more static glib constants left, so this can be closed now.
Comment 12 Simon Feltman 2012-11-05 10:26:14 UTC
Review of attachment 228088 [details] [review]:

Even better.