GNOME Bugzilla – Bug 642607
Can't use an int in a flag type
Last modified: 2011-02-22 11:19:05 UTC
If I have a method with a flag parameter and I do something like flagname1 | flagname2 it doesn't work and says that it cannot be passed an int. If I pass an int doesn't work either, only if I pass flagname1
Martin was having this same issue awhile back and I think it turned out to be something he was doing wrong. Flags are checked to make sure you don't send an invalid flag value. You can't send an int other than 0. Concatenating 2 flags with | should show up as a new flag type e.g. <Foo.Flags(Foo.Flags.flagname1 | Foo.Flags.flagname2)>. Can you post a code snipit of the flags that are not working?
See http://git.gnome.org/browse/gedit-plugins/tree/plugins/terminal/terminal.py#n90 I wanted to make: GLib.SpawnFlags.SEARCH_PATH | GLib.SpawnFlags.CHILD_INHERITS_STDIN and it doesn't work. Also see that I have to use GLib introspected, using the flags from import glib doesn't work either.
This fails for flags with no gtype. All Gtk flags have a gtype, but Gst doesn't use gtypes for any of it's flags or enums. You can reproduce this bug using Gst: >>> from gi.repository import Gst >>> Gst.PluginFlags.CACHED <enum CACHED of type Gst.PluginFlags> >>> Gst.PluginFlags.BLACKLISTED <enum BLACKLISTED of type Gst.PluginFlags> >>> Gst.PluginFlags.CACHED | Gst.PluginFlags.BLACKLISTED 3 If a flag has no gtype, it is being wrapped as an enum object. As enum is a subclass of int, the | operator returns an int.
The | operator method for flags should get invoked but it looks like it is listing them as enums instead of flags. Gst.PluginFlags(Gst.PluginFlags.CACHED | Gst.PluginFlags.BLACKLISTED) might workaround your issue for now. Not sure why it can't take an integer if it thinks it is an enum. Same goes for the GLib ones. Enum's shouldn't have the flag checks.
Created attachment 181362 [details] [review] [GI] Register GType for non-GType enums and flags at runtime. Previously non-GType enums used a separate type implemented in Python, and non-GType flags had no implementation at all. This removes the separate type for enums, and registers a new GType at runtime if there isn't one. This allows non-GType enums and flags to use the same Python type as GType enums and flags. This removes duplication of code, and make both kinds behave identically. identical, and reducing duplication of code.
There is one problem even with my patch applied. The dynamic GObject module has a __getattr__() which allows access to attrs from static gobject. With introspection, flags are supposed to be accessed like GObject.SignalFlags.ACTION, but because of that __getattr__() GObject.SIGNAL_ACTION is still available. We cannot remove SIGNAL_ACTION from gobject, because that would break the API for static Python applications. But gobject.SIGNAL_ACTION is installed on the module as an int with PyModule_AddIntConstant() and when passing it to a function, you get an error saying it is an int, not type SignalFlags. As long as people using introspection always use GObject.SignalFlags.ACTION instead of GObject.SIGNAL_ACTION there is no problem. But having the latter available will cause confusion for people porting from static bindings. There are other flags in glib/gobject which have the same issue: IOFlags, OptionFlags, SpawnFlags, etc.
Comment on attachment 181362 [details] [review] [GI] Register GType for non-GType enums and flags at runtime. >From 01903f6d3a434af1713a7ce141a52979e0915fb1 Mon Sep 17 00:00:00 2001 >From: Laszlo Pandy <lpandy@src.gnome.org> >Date: Sat, 19 Feb 2011 23:11:25 +0100 >Subject: [PATCH] [GI] Register GType for non-GType enums and flags at runtime. > >Previously non-GType enums used a separate type implemented in >Python, and non-GType flags had no implementation at all. This >removes the separate type for enums, and registers a new GType at >runtime if there isn't one. > >This allows non-GType enums and flags to use the same Python type >as GType enums and flags. This removes duplication of code, and >make both kinds behave identically. > >identical, and reducing duplication of code. > >https://bugzilla.gnome.org/show_bug.cgi?id=642607 >--- > gi/gimodule.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > gi/module.py | 24 +++++++---- > gi/pygi-info.c | 15 +++++++ > gi/types.py | 16 ------- > gobject/pygflags.c | 13 ++++-- > 5 files changed, 156 insertions(+), 29 deletions(-) > >diff --git a/gi/gimodule.c b/gi/gimodule.c >index 55c9d8b..72047d4 100644 >--- a/gi/gimodule.c >+++ b/gi/gimodule.c >@@ -51,6 +51,63 @@ _wrap_pyg_enum_add (PyObject *self, > } > > static PyObject * should most likely be _wrap_pyg_enum_register_new_gtype >+_wrap_pyg_enum_add_make_new_gtype (PyObject *self, >+ PyObject *args, >+ PyObject *kwargs) >+{ >+ static char *kwlist[] = { "info", NULL }; >+ PyGIBaseInfo *py_info; >+ GIEnumInfo *info; >+ gint n_values; >+ GEnumValue *g_enum_values; >+ GType g_type; >+ const gchar *type_name; >+ >+ if (!PyArg_ParseTupleAndKeywords (args, kwargs, >+ "O:enum_add_make_new_gtype", >+ kwlist, (PyObject *)&py_info)) { >+ return NULL; >+ } >+ >+ if (!GI_IS_ENUM_INFO (py_info->info) || >+ g_base_info_get_type ((GIBaseInfo *) py_info->info) != GI_INFO_TYPE_ENUM) { >+ PyErr_SetString (PyExc_TypeError, "info must be an EnumInfo with info type GI_INFO_TYPE_ENUM"); >+ return NULL; >+ } >+ >+ info = (GIEnumInfo *)py_info->info; >+ n_values = g_enum_info_get_n_values (info); >+ g_enum_values = g_new0 (GEnumValue, n_values + 1); >+ >+ for (int i=0; i < n_values; i++) { >+ GIValueInfo *value_info; >+ GEnumValue *enum_value; >+ const gchar *name; >+ >+ value_info = g_enum_info_get_value (info, i); >+ name = g_base_info_get_name ((GIBaseInfo *) value_info); >+ >+ enum_value = &g_enum_values[i]; >+ enum_value->value_nick = g_strdup (name); >+ /* TODO: get "c:identifier" attribute for value_name once GI exposes it in the typelib */ >+ enum_value->value_name = enum_value->value_nick; >+ enum_value->value = g_value_info_get_value (value_info); >+ >+ g_base_info_unref ((GIBaseInfo *) value_info); >+ } >+ >+ g_enum_values[n_values].value = 0; >+ g_enum_values[n_values].value_nick = NULL; >+ g_enum_values[n_values].value_name = NULL; >+ >+ type_name = g_base_info_get_name ((GIBaseInfo *) info); >+ type_name = g_strdup (type_name); >+ g_type = g_enum_register_static (type_name, g_enum_values); >+ >+ return pyg_enum_add (NULL, g_type_name (g_type), NULL, g_type); >+} >+ >+static PyObject * > _wrap_pyg_flags_add (PyObject *self, > PyObject *args, > PyObject *kwargs) >@@ -74,6 +131,64 @@ _wrap_pyg_flags_add (PyObject *self, > } > > static PyObject * should most likely be _wrap_pyg_flag_register_new_gtype >+_wrap_pyg_flags_add_make_new_gtype (PyObject *self, >+ PyObject *args, >+ PyObject *kwargs) >+{ >+ static char *kwlist[] = { "info", NULL }; >+ PyGIBaseInfo *py_info; >+ GIEnumInfo *info; >+ gint n_values; >+ GFlagsValue *g_flags_values; >+ GType g_type; >+ const gchar *type_name; >+ >+ if (!PyArg_ParseTupleAndKeywords (args, kwargs, >+ "O:flags_add_make_new_gtype", >+ kwlist, (PyObject *)&py_info)) { >+ return NULL; >+ } >+ >+ if (!GI_IS_ENUM_INFO (py_info->info) || >+ g_base_info_get_type ((GIBaseInfo *) py_info->info) != GI_INFO_TYPE_FLAGS) { >+ PyErr_SetString (PyExc_TypeError, "info must be an EnumInfo with info type GI_INFO_TYPE_FLAGS"); >+ return NULL; >+ } >+ >+ info = (GIEnumInfo *)py_info->info; >+ n_values = g_enum_info_get_n_values (info); >+ g_flags_values = g_new0 (GFlagsValue, n_values + 1); >+ >+ for (int i=0; i < n_values; i++) { >+ GIValueInfo *value_info; >+ GFlagsValue *flags_value; >+ const gchar *name; >+ >+ value_info = g_enum_info_get_value (info, i); >+ name = g_base_info_get_name ((GIBaseInfo *) value_info); >+ >+ flags_value = &g_flags_values[i]; >+ flags_value->value_nick = g_strdup (name); >+ /* TODO: get "c:identifier" attribute for value_name once GI exposes it in the typelib */ >+ flags_value->value_name = flags_value->value_nick; >+ flags_value->value = g_value_info_get_value (value_info); >+ >+ g_base_info_unref ((GIBaseInfo *) value_info); >+ } >+ >+ g_flags_values[n_values].value = 0; >+ g_flags_values[n_values].value_nick = NULL; >+ g_flags_values[n_values].value_name = NULL; >+ >+ type_name = g_base_info_get_name ((GIBaseInfo *) info); >+ type_name = g_strdup (type_name); >+ g_type = g_flags_register_static (type_name, g_flags_values); >+ >+ return pyg_flags_add (NULL, g_type_name (g_type), NULL, g_type); >+} >+ >+ >+static PyObject * > _wrap_pyg_set_object_has_new_constructor (PyObject *self, > PyObject *args, > PyObject *kwargs) >@@ -353,7 +468,9 @@ _wrap_pyg_variant_type_from_string (PyObject *self, PyObject *args) > > static PyMethodDef _gi_functions[] = { > { "enum_add", (PyCFunction) _wrap_pyg_enum_add, METH_VARARGS | METH_KEYWORDS }, >+ { "enum_add_make_new_gtype", (PyCFunction) _wrap_pyg_enum_add_make_new_gtype, METH_VARARGS | METH_KEYWORDS }, > { "flags_add", (PyCFunction) _wrap_pyg_flags_add, METH_VARARGS | METH_KEYWORDS }, >+ { "flags_add_make_new_gtype", (PyCFunction) _wrap_pyg_flags_add_make_new_gtype, METH_VARARGS | METH_KEYWORDS }, > > { "set_object_has_new_constructor", (PyCFunction) _wrap_pyg_set_object_has_new_constructor, METH_VARARGS | METH_KEYWORDS }, > { "register_interface_info", (PyCFunction) _wrap_pyg_register_interface_info, METH_VARARGS }, >diff --git a/gi/module.py b/gi/module.py >index 9b935ed..f684aed 100644 >--- a/gi/module.py >+++ b/gi/module.py >@@ -40,12 +40,13 @@ from ._gi import \ > Struct, \ > Boxed, \ > enum_add, \ >- flags_add >+ enum_add_make_new_gtype, \ >+ flags_add, \ >+ flags_add_make_new_gtype > from .types import \ > GObjectMeta, \ > StructMeta, \ >- Function, \ >- Enum >+ Function > > repository = Repository.get_default() > >@@ -102,13 +103,18 @@ class IntrospectionModule(object): > wrapper = g_type.pytype > > if wrapper is None: >- if g_type.is_a(gobject.TYPE_ENUM): >- wrapper = enum_add(g_type) >- elif g_type.is_a(gobject.TYPE_NONE): >- # An enum with a GType of None is an enum without GType >- wrapper = type(info.get_name(), (Enum,), {}) >+ if info.is_flags(): >+ if g_type.is_a(gobject.TYPE_FLAGS): >+ wrapper = flags_add(g_type) >+ else: >+ assert g_type == gobject.TYPE_NONE >+ wrapper = flags_add_make_new_gtype(info) > else: >- wrapper = flags_add(g_type) >+ if g_type.is_a(gobject.TYPE_ENUM): >+ wrapper = enum_add(g_type) >+ else: >+ assert g_type == gobject.TYPE_NONE >+ wrapper = enum_add_make_new_gtype(info) > > wrapper.__info__ = info > wrapper.__module__ = 'gi.repository.' + info.get_namespace() >diff --git a/gi/pygi-info.c b/gi/pygi-info.c >index f5dd69f..1bfd7d8 100644 >--- a/gi/pygi-info.c >+++ b/gi/pygi-info.c >@@ -923,8 +923,23 @@ _wrap_g_enum_info_get_values (PyGIBaseInfo *self) > return infos; > } > >+static PyObject * >+_wrap_g_enum_info_is_flags (PyGIBaseInfo *self) >+{ >+ GIInfoType info_type = g_base_info_get_type ((GIBaseInfo *) self->info); >+ >+ if (info_type == GI_INFO_TYPE_ENUM) { >+ Py_RETURN_FALSE; >+ } else if (info_type == GI_INFO_TYPE_FLAGS) { >+ Py_RETURN_TRUE; >+ } else { >+ g_assert_not_reached(); >+ } >+} >+ > static PyMethodDef _PyGIEnumInfo_methods[] = { > { "get_values", (PyCFunction) _wrap_g_enum_info_get_values, METH_NOARGS }, >+ { "is_flags", (PyCFunction) _wrap_g_enum_info_is_flags, METH_NOARGS }, > { NULL, NULL, 0 } > }; > >diff --git a/gi/types.py b/gi/types.py >index 37cf499..c2a8b35 100644 >--- a/gi/types.py >+++ b/gi/types.py >@@ -250,19 +250,3 @@ class StructMeta(type, MetaClassHelper): > not method_info.get_arguments(): > cls.__new__ = staticmethod(Constructor(method_info)) > break >- >-class Enum(int): >- # Only subclasses of this type should be instantiated. >- # Each subclass requires an __info__ attribute, >- # which is not declared here because enums do not share the same gi type. >- def __init__(self, value): >- int.__init__(value) >- >- def __repr__(self): >- value_name = str(self) >- for value_info in self.__info__.get_values(): >- if self == value_info.get_value(): >- value_name = value_info.get_name().upper() >- return "<enum %s of type %s.%s>" % (value_name, >- self.__info__.get_namespace(), >- self.__info__.get_name()) >diff --git a/gobject/pygflags.c b/gobject/pygflags.c >index 936f314..ce99a86 100644 >--- a/gobject/pygflags.c >+++ b/gobject/pygflags.c >@@ -172,13 +172,18 @@ pyg_flags_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) > > pyint = PYGLIB_PyLong_FromLong(value); > ret = PyDict_GetItem(values, pyint); >+ if (!ret) { >+ PyErr_Clear(); >+ >+ ret = pyg_flags_val_new((PyObject *)type, gtype, pyint); >+ g_assert(ret != NULL); >+ } else { >+ Py_INCREF(ret); >+ } >+ > Py_DECREF(pyint); > Py_DECREF(values); > >- if (ret) >- Py_INCREF(ret); >- else >- PyErr_Format(PyExc_ValueError, "invalid flag value: %ld", value); > return ret; > } > >-- >1.7.1
Just a couple of naming issues but ok for commit!