[pygobject] Add support for variable user data arguments
- From: Simon Feltman <sfeltman src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [pygobject] Add support for variable user data arguments
- Date: Mon, 14 Oct 2013 21:52:20 +0000 (UTC)
commit 9456e83233a927f1f01c6ffcb1f07c62b491a1df
Author: Simon Feltman <sfeltman src gnome org>
Date: Wed Aug 7 12:08:15 2013 -0700
Add support for variable user data arguments
Support a variable number of user data arguments for all callback
connection function where the user data is the last explicit argument.
This adds convience as well as consistency with the rest of PyGObject.
Cleanup overrides for GLib.idle_add, timeout_add, timeout_add_seconds,
io_add_watch, and child_watch_add which manually implemented this
feature.
https://bugzilla.gnome.org/show_bug.cgi?id=640812
gi/overrides/GLib.py | 75 ++++++++++++++-------------------------------
gi/pygi-cache.c | 14 ++++++++
gi/pygi-cache.h | 3 ++
gi/pygi-closure.c | 37 ++++++++++++++--------
gi/pygi-invoke.c | 71 ++++++++++++++++++++++++++++++------------
gi/pygi-marshal-from-py.c | 5 +++
tests/test_everything.py | 32 +++++++++++++++++++
tests/test_glib.py | 21 ++++++++++++
tests/test_subprocess.py | 8 ++--
9 files changed, 177 insertions(+), 89 deletions(-)
---
diff --git a/gi/overrides/GLib.py b/gi/overrides/GLib.py
index 672957c..ddd65bd 100644
--- a/gi/overrides/GLib.py
+++ b/gi/overrides/GLib.py
@@ -617,47 +617,24 @@ class Timeout(Source):
__all__.append('Timeout')
-def user_data_varargs_shim(callback, user_data, cb_num_args=0):
- '''Adjust callback and user_data varargs for PyGTK backwards compatibility
-
- GLib only accepts exactly one user_data argument, but older pygtk
- traditionally accepted zero or more for some specific functions. For "one
- argument", use the actual user-supplied callback for efficiency; for all
- others, rewire it to accept zero or more than one.
-
- Return the adjusted callback and the real user data to pass to GLib.
- '''
- if len(user_data) == 1:
- return (callback, user_data[0])
-
- if cb_num_args == 0:
- return (lambda data: callback(*data), user_data)
- if cb_num_args == 2:
- return (lambda a1, a2, data: callback(a1, a2, *data), user_data)
- raise NotImplementedError('%i number of callback arguments not supported' % cb_num_args)
-
-
# backwards compatible API
def idle_add(function, *user_data, **kwargs):
- (fn, data) = user_data_varargs_shim(function, user_data)
priority = kwargs.get('priority', GLib.PRIORITY_DEFAULT_IDLE)
- return GLib.idle_add(priority, fn, data)
+ return GLib.idle_add(priority, function, *user_data)
__all__.append('idle_add')
def timeout_add(interval, function, *user_data, **kwargs):
- (fn, data) = user_data_varargs_shim(function, user_data)
priority = kwargs.get('priority', GLib.PRIORITY_DEFAULT)
- return GLib.timeout_add(priority, interval, fn, data)
+ return GLib.timeout_add(priority, interval, function, *user_data)
__all__.append('timeout_add')
def timeout_add_seconds(interval, function, *user_data, **kwargs):
- (fn, data) = user_data_varargs_shim(function, user_data)
priority = kwargs.get('priority', GLib.PRIORITY_DEFAULT)
- return GLib.timeout_add_seconds(priority, interval, fn, data)
+ return GLib.timeout_add_seconds(priority, interval, function, *user_data)
__all__.append('timeout_add_seconds')
@@ -674,8 +651,6 @@ __all__.append('timeout_add_seconds')
# - calling with a Python file object as first argument (we keep this one as
# it's really convenient and does not change the number of arguments)
# - calling without a priority as second argument
-# and the usual "call without or multiple user_data", in which case the
-# callback gets the same user data arguments.
def _io_add_watch_get_args(channel, priority_, condition, *cb_and_user_data, **kwargs):
if not isinstance(priority_, int) or isinstance(priority_, GLib.IOCondition):
warnings.warn('Calling io_add_watch without priority as second argument is deprecated',
@@ -700,19 +675,17 @@ def _io_add_watch_get_args(channel, priority_, condition, *cb_and_user_data, **k
callback = cb_and_user_data[0]
user_data = cb_and_user_data[1:]
- (func, user_data) = user_data_varargs_shim(callback, user_data, 2)
-
# backwards compatibility: Allow calling with fd
if isinstance(channel, int):
- func_fdtransform = lambda _, cond, data: func(channel, cond, data)
+ func_fdtransform = lambda _, cond, *data: callback(channel, cond, *data)
real_channel = GLib.IOChannel.unix_new(channel)
elif hasattr(channel, 'fileno'):
# backwards compatibility: Allow calling with Python file
- func_fdtransform = lambda _, cond, data: func(channel, cond, data)
+ func_fdtransform = lambda _, cond, *data: callback(channel, cond, *data)
real_channel = GLib.IOChannel.unix_new(channel.fileno())
else:
assert isinstance(channel, GLib.IOChannel)
- func_fdtransform = func
+ func_fdtransform = callback
real_channel = channel
return real_channel, priority_, condition, func_fdtransform, user_data
@@ -723,7 +696,7 @@ __all__.append('_io_add_watch_get_args')
def io_add_watch(*args, **kwargs):
"""io_add_watch(channel, priority, condition, func, *user_data) -> event_source_id"""
channel, priority, condition, func, user_data = _io_add_watch_get_args(*args, **kwargs)
- return GLib.io_add_watch(channel, priority, condition, func, user_data)
+ return GLib.io_add_watch(channel, priority, condition, func, *user_data)
__all__.append('io_add_watch')
@@ -828,7 +801,7 @@ __all__.append('PollFD')
# We need to support this until we are okay with breaking API in a way which is
# not backwards compatible.
def _child_watch_add_get_args(priority_or_pid, pid_or_callback, *args, **kwargs):
- _unspecified = object()
+ user_data = []
if callable(pid_or_callback):
warnings.warn('Calling child_watch_add without priority as first argument is deprecated',
@@ -836,35 +809,33 @@ def _child_watch_add_get_args(priority_or_pid, pid_or_callback, *args, **kwargs)
pid = priority_or_pid
callback = pid_or_callback
if len(args) == 0:
- user_data = kwargs.get('data', _unspecified)
priority = kwargs.get('priority', GLib.PRIORITY_DEFAULT)
elif len(args) == 1:
- user_data = args[0]
+ user_data = args
priority = kwargs.get('priority', GLib.PRIORITY_DEFAULT)
elif len(args) == 2:
- user_data = args[0]
+ user_data = [args[0]]
priority = args[1]
else:
raise TypeError('expected at most 4 positional arguments')
else:
priority = priority_or_pid
pid = pid_or_callback
- if len(args) == 0 or not callable(args[0]):
- raise TypeError('expected callback as third argument')
- callback = args[0]
- if len(args) == 1:
- user_data = kwargs.get('data', _unspecified)
+ if 'function' in kwargs:
+ callback = kwargs['function']
+ user_data = args
+ elif len(args) > 0 and callable(args[0]):
+ callback = args[0]
+ user_data = args[1:]
else:
- user_data = args[1]
+ raise TypeError('expected callback as third argument')
- if user_data is _unspecified:
- # we have to call the callback without the user_data argument
- func = lambda pid, status, data: callback(pid, status)
- user_data = None
- else:
- func = callback
+ if 'data' in kwargs:
+ if user_data:
+ raise TypeError('got multiple values for "data" argument')
+ user_data = [kwargs['data']]
- return priority, pid, func, user_data
+ return priority, pid, callback, user_data
# we need this to be accessible for unit testing
__all__.append('_child_watch_add_get_args')
@@ -873,7 +844,7 @@ __all__.append('_child_watch_add_get_args')
def child_watch_add(*args, **kwargs):
"""child_watch_add(priority, pid, function, *data)"""
priority, pid, function, data = _child_watch_add_get_args(*args, **kwargs)
- return GLib.child_watch_add(priority, pid, function, data)
+ return GLib.child_watch_add(priority, pid, function, *data)
__all__.append('child_watch_add')
diff --git a/gi/pygi-cache.c b/gi/pygi-cache.c
index 8459449..14d69b1 100644
--- a/gi/pygi-cache.c
+++ b/gi/pygi-cache.c
@@ -1121,6 +1121,9 @@ _args_cache_generate (GICallableInfo *callable_info,
g_hash_table_remove_all (callable_cache->arg_name_hash);
}
callable_cache->n_py_required_args = 0;
+ callable_cache->user_data_varargs_index = -1;
+
+ gssize last_explicit_arg_index = -1;
/* Reverse loop through all the arguments to setup arg_name_list/hash
* and find the number of required arguments */
@@ -1151,6 +1154,17 @@ _args_cache_generate (GICallableInfo *callable_info,
} else if (!arg_cache->has_default) {
callable_cache->n_py_required_args += 1;
}
+
+ if (last_explicit_arg_index == -1) {
+ last_explicit_arg_index = i;
+
+ /* If the last "from python" argument in the args list is a child
+ * with pyarg (currently only callback user_data). Set it to eat
+ * variable args in the callable cache.
+ */
+ if (arg_cache->meta_type == PYGI_META_ARG_TYPE_CHILD_WITH_PYARG)
+ callable_cache->user_data_varargs_index = i;
+ }
}
}
diff --git a/gi/pygi-cache.h b/gi/pygi-cache.h
index 24a8406..1b32381 100644
--- a/gi/pygi-cache.h
+++ b/gi/pygi-cache.h
@@ -177,6 +177,9 @@ struct _PyGICallableCache
GSList *arg_name_list; /* for keyword arg matching */
GHashTable *arg_name_hash;
+ /* Index of user_data arg that can eat variable args passed to a callable. */
+ gssize user_data_varargs_index;
+
/* Number of in args passed to g_function_info_invoke.
* This is used for the length of PyGIInvokeState.in_args */
gssize n_from_py_args;
diff --git a/gi/pygi-closure.c b/gi/pygi-closure.c
index bfd6d0d..5df4713 100644
--- a/gi/pygi-closure.c
+++ b/gi/pygi-closure.c
@@ -368,23 +368,34 @@ _pygi_closure_convert_arguments (GICallableInfo *callable_info, void **args,
if (direction == GI_DIRECTION_IN && arg_tag == GI_TYPE_TAG_VOID &&
g_type_info_is_pointer (&arg_type)) {
- if (user_data == _PyGIDefaultArgPlaceholder) {
- /* When user data is a place holder, skip handing anything to the callback.
- * This happens when the callback connect function accepts user data
- * but nothing was passed in.
- */
- continue;
- } else if (user_data == NULL) {
- /* user data can be NULL for connect functions which don't accept
- * user data but we still need to pass None to the callbacks for
- * compatibility of setups prior to _PyGIDefaultArgPlaceholder.
- * For example: Regress.test_async_ready_callback
+ if (user_data == NULL) {
+ /* user_data can be NULL for connect functions which don't accept
+ * user_data or as the default for user_data in the middle of function
+ * arguments.
*/
Py_INCREF (Py_None);
value = Py_None;
} else {
- value = user_data;
- Py_INCREF (value);
+ /* Extend the callbacks args with user_data as variable args. */
+ int j, user_data_len;
+ PyObject *py_user_data = user_data;
+
+ if (!PyTuple_Check (py_user_data)) {
+ PyErr_SetString (PyExc_TypeError, "expected tuple for callback user_data");
+ goto error;
+ }
+
+ user_data_len = PyTuple_Size (py_user_data);
+ _PyTuple_Resize (py_args, n_args + user_data_len - 1);
+ for (j = 0; j < user_data_len; j++, n_in_args++) {
+ value = PyTuple_GetItem (py_user_data, j);
+ Py_INCREF (value);
+ PyTuple_SET_ITEM (*py_args, n_in_args, value);
+ }
+ /* We can assume user_data args are never going to be inout,
+ * so just continue here.
+ */
+ continue;
}
} else if (direction == GI_DIRECTION_IN &&
arg_tag == GI_TYPE_TAG_INTERFACE) {
diff --git a/gi/pygi-invoke.c b/gi/pygi-invoke.c
index eab6c2e..9dc40d6 100644
--- a/gi/pygi-invoke.c
+++ b/gi/pygi-invoke.c
@@ -165,12 +165,12 @@ _py_args_combine_and_check_length (PyGICallableCache *cache,
/* Fast path, we already have the exact number of args and not kwargs. */
n_expected_args = g_slist_length (cache->arg_name_list);
- if (n_py_kwargs == 0 && n_py_args == n_expected_args) {
+ if (n_py_kwargs == 0 && n_py_args == n_expected_args && cache->user_data_varargs_index < 0) {
Py_INCREF (py_args);
return py_args;
}
- if (n_expected_args < n_py_args) {
+ if (cache->user_data_varargs_index < 0 && n_expected_args < n_py_args) {
PyErr_Format (PyExc_TypeError,
"%.200s() takes exactly %d %sargument%s (%zd given)",
function_name,
@@ -181,6 +181,13 @@ _py_args_combine_and_check_length (PyGICallableCache *cache,
return NULL;
}
+ if (cache->user_data_varargs_index >= 0 && n_py_kwargs > 0 && n_expected_args < n_py_args) {
+ PyErr_Format (PyExc_TypeError,
+ "%.200s() cannot use variable user data arguments with keyword arguments",
+ function_name);
+ return NULL;
+ }
+
if (n_py_kwargs > 0 && !_check_for_unexpected_kwargs (function_name,
cache->arg_name_hash,
py_kwargs)) {
@@ -191,35 +198,59 @@ _py_args_combine_and_check_length (PyGICallableCache *cache,
* when they are combined into a single tuple */
combined_py_args = PyTuple_New (n_expected_args);
- for (i = 0; i < n_py_args; i++) {
- PyObject *item = PyTuple_GET_ITEM (py_args, i);
- Py_INCREF (item);
- PyTuple_SET_ITEM (combined_py_args, i, item);
- }
-
for (i = 0, l = cache->arg_name_list; i < n_expected_args && l; i++, l = l->next) {
- PyObject *py_arg_item, *kw_arg_item = NULL;
+ PyObject *py_arg_item = NULL;
+ PyObject *kw_arg_item = NULL;
const gchar *arg_name = l->data;
+ int arg_cache_index = -1;
+ gboolean is_varargs_user_data = FALSE;
+
+ if (arg_name != NULL)
+ arg_cache_index = GPOINTER_TO_INT (g_hash_table_lookup (cache->arg_name_hash, arg_name));
+
+ is_varargs_user_data = cache->user_data_varargs_index >= 0 &&
+ arg_cache_index == cache->user_data_varargs_index;
if (n_py_kwargs > 0 && arg_name != NULL) {
/* NULL means this argument has no keyword name */
/* ex. the first argument to a method or constructor */
kw_arg_item = PyDict_GetItemString (py_kwargs, arg_name);
}
- py_arg_item = PyTuple_GET_ITEM (combined_py_args, i);
- if (kw_arg_item != NULL && py_arg_item == NULL) {
- Py_INCREF (kw_arg_item);
- PyTuple_SET_ITEM (combined_py_args, i, kw_arg_item);
+ /* use a bounded retrieval of the original input */
+ if (i < n_py_args)
+ py_arg_item = PyTuple_GET_ITEM (py_args, i);
+
+ if (kw_arg_item == NULL && py_arg_item != NULL) {
+ if (is_varargs_user_data) {
+ /* For tail end user_data varargs, pull a slice off and we are done. */
+ PyObject *user_data = PyTuple_GetSlice (py_args, i, PY_SSIZE_T_MAX);
+ PyTuple_SET_ITEM (combined_py_args, i, user_data);
+ return combined_py_args;
+ } else {
+ Py_INCREF (py_arg_item);
+ PyTuple_SET_ITEM (combined_py_args, i, py_arg_item);
+ }
+ } else if (kw_arg_item != NULL && py_arg_item == NULL) {
+ if (is_varargs_user_data) {
+ /* Special case where user_data is passed as a keyword argument (user_data=foo)
+ * Wrap the value in a tuple to represent variable args for marshaling later on.
+ */
+ PyObject *user_data = Py_BuildValue("(O)", kw_arg_item, NULL);
+ PyTuple_SET_ITEM (combined_py_args, i, user_data);
+ } else {
+ Py_INCREF (kw_arg_item);
+ PyTuple_SET_ITEM (combined_py_args, i, kw_arg_item);
+ }
} else if (kw_arg_item == NULL && py_arg_item == NULL) {
- /* If the argument supports a default, use a place holder in the
- * argument tuple, this will be checked later during marshaling.
- */
- int arg_cache_index = -1;
- if (arg_name != NULL)
- arg_cache_index = GPOINTER_TO_INT (g_hash_table_lookup (cache->arg_name_hash, arg_name));
- if (arg_cache_index >= 0 && _pygi_callable_cache_get_arg (cache, arg_cache_index)->has_default) {
+ if (is_varargs_user_data) {
+ /* For varargs user_data, pass an empty tuple when nothing is given. */
+ PyTuple_SET_ITEM (combined_py_args, i, PyTuple_New (0));
+ } else if (arg_cache_index >= 0 && _pygi_callable_cache_get_arg (cache,
arg_cache_index)->has_default) {
+ /* If the argument supports a default, use a place holder in the
+ * argument tuple, this will be checked later during marshaling.
+ */
Py_INCREF (_PyGIDefaultArgPlaceholder);
PyTuple_SET_ITEM (combined_py_args, i, _PyGIDefaultArgPlaceholder);
} else {
diff --git a/gi/pygi-marshal-from-py.c b/gi/pygi-marshal-from-py.c
index f257ecd..df22863 100644
--- a/gi/pygi-marshal-from-py.c
+++ b/gi/pygi-marshal-from-py.c
@@ -1262,6 +1262,11 @@ _pygi_marshal_from_py_interface_callback (PyGIInvokeState *state,
py_user_data = PyTuple_GetItem (state->py_in_args, user_data_cache->py_arg_index);
if (!py_user_data)
return FALSE;
+ /* NULL out user_data if it was not supplied and the default arg placeholder
+ * was used instead.
+ */
+ if (py_user_data == _PyGIDefaultArgPlaceholder)
+ py_user_data = NULL;
}
}
diff --git a/tests/test_everything.py b/tests/test_everything.py
index ab1fc48..5c4ba6b 100644
--- a/tests/test_everything.py
+++ b/tests/test_everything.py
@@ -730,6 +730,38 @@ class TestCallbacks(unittest.TestCase):
self.assertEqual(TestCallbacks.called, 100)
+ def test_callback_userdata_varargs(self):
+ TestCallbacks.called = 0
+ collected_user_data = []
+
+ def callback(a, b):
+ collected_user_data.extend([a, b])
+ TestCallbacks.called += 1
+ return TestCallbacks.called
+
+ for i in range(10):
+ val = Everything.test_callback_user_data(callback, 1, 2)
+ self.assertEqual(val, i + 1)
+
+ self.assertEqual(TestCallbacks.called, 10)
+ self.assertSequenceEqual(collected_user_data, [1, 2] * 10)
+
+ def test_callback_userdata_as_kwarg_tuple(self):
+ TestCallbacks.called = 0
+ collected_user_data = []
+
+ def callback(user_data):
+ collected_user_data.extend(user_data)
+ TestCallbacks.called += 1
+ return TestCallbacks.called
+
+ for i in range(10):
+ val = Everything.test_callback_user_data(callback, user_data=(1, 2))
+ self.assertEqual(val, i + 1)
+
+ self.assertEqual(TestCallbacks.called, 10)
+ self.assertSequenceEqual(collected_user_data, [1, 2] * 10)
+
def test_async_ready_callback(self):
TestCallbacks.called = False
TestCallbacks.main_loop = GLib.MainLoop()
diff --git a/tests/test_glib.py b/tests/test_glib.py
index 3cde168..a01e83e 100644
--- a/tests/test_glib.py
+++ b/tests/test_glib.py
@@ -165,6 +165,27 @@ https://my.org/q?x=1&y=2
self.assertEqual(call_data, [(r, GLib.IOCondition.IN, b'a', 'moo'),
(r, GLib.IOCondition.IN, b'b', 'moo')])
+ def test_io_add_watch_with_multiple_data(self):
+ (r, w) = os.pipe()
+ call_data = []
+
+ def cb(fd, condition, *user_data):
+ call_data.append((fd, condition, os.read(fd, 1), user_data))
+ return True
+
+ # io_add_watch() takes an IOChannel, calling with an fd is deprecated
+ with warnings.catch_warnings(record=True) as warn:
+ warnings.simplefilter('always')
+ GLib.io_add_watch(r, GLib.IOCondition.IN, cb, 'moo', 'foo')
+ self.assertTrue(issubclass(warn[0].category, PyGIDeprecationWarning))
+
+ ml = GLib.MainLoop()
+ GLib.timeout_add(10, lambda: os.write(w, b'a') and False)
+ GLib.timeout_add(100, ml.quit)
+ ml.run()
+
+ self.assertEqual(call_data, [(r, GLib.IOCondition.IN, b'a', ('moo', 'foo'))])
+
def test_io_add_watch_pyfile(self):
call_data = []
diff --git a/tests/test_subprocess.py b/tests/test_subprocess.py
index cf5ef04..6da8ad2 100644
--- a/tests/test_subprocess.py
+++ b/tests/test_subprocess.py
@@ -23,7 +23,7 @@ class TestProcess(unittest.TestCase):
self.assertEqual(res[0], GLib.PRIORITY_DEFAULT)
self.assertEqual(res[1], pid)
self.assertTrue(callable(cb))
- self.assertEqual(res[3], None)
+ self.assertSequenceEqual(res[3], [])
def test_deprecated_child_watch_data_priority(self):
cb = lambda pid, status: None
@@ -37,7 +37,7 @@ class TestProcess(unittest.TestCase):
self.assertEqual(res[0], GLib.PRIORITY_HIGH)
self.assertEqual(res[1], pid)
self.assertEqual(res[2], cb)
- self.assertEqual(res[3], 12345)
+ self.assertSequenceEqual(res[3], [12345])
def test_deprecated_child_watch_data_priority_kwargs(self):
cb = lambda pid, status: None
@@ -51,7 +51,7 @@ class TestProcess(unittest.TestCase):
self.assertEqual(res[0], GLib.PRIORITY_HIGH)
self.assertEqual(res[1], pid)
self.assertEqual(res[2], cb)
- self.assertEqual(res[3], 12345)
+ self.assertSequenceEqual(res[3], [12345])
@unittest.expectedFailure # using keyword args is fully supported by PyGObject machinery
def test_child_watch_all_kwargs(self):
@@ -63,7 +63,7 @@ class TestProcess(unittest.TestCase):
self.assertEqual(res[0], GLib.PRIORITY_HIGH)
self.assertEqual(res[1], pid)
self.assertEqual(res[2], cb)
- self.assertEqual(res[3], 12345)
+ self.assertSequenceEqual(res[3], [12345])
def test_child_watch_no_data(self):
def cb(pid, status):
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
