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 642607 - Can't use an int in a flag type
Can't use an int in a flag type
Status: RESOLVED FIXED
Product: pygobject
Classification: Bindings
Component: introspection
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Nobody's working on this now (help wanted and appreciated)
Python bindings maintainers
Depends on:
Blocks: 642922
 
 
Reported: 2011-02-17 18:46 UTC by Ignacio Casal Quinteiro (nacho)
Modified: 2011-02-22 11:19 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[GI] Register GType for non-GType enums and flags at runtime. (10.32 KB, patch)
2011-02-19 22:19 UTC, Laszlo Pandy
committed Details | Review

Description Ignacio Casal Quinteiro (nacho) 2011-02-17 18:46:26 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
Comment 1 johnp 2011-02-17 19:53:48 UTC
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?
Comment 2 Ignacio Casal Quinteiro (nacho) 2011-02-18 10:08:12 UTC
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.
Comment 3 Laszlo Pandy 2011-02-18 11:11:04 UTC
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.
Comment 4 johnp 2011-02-18 19:34:54 UTC
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.
Comment 5 Laszlo Pandy 2011-02-19 22:19:58 UTC
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.
Comment 6 Laszlo Pandy 2011-02-20 15:55:12 UTC
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 7 johnp 2011-02-21 22:52:06 UTC
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
Comment 8 johnp 2011-02-21 22:52:36 UTC
Just a couple of naming issues but ok for commit!