classification
Title: **kwargs unnecessarily restricted in concurrent.futures 'submit' API
Type: behavior Stage:
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: bquinlan Nosy List: avdd, bquinlan, georg.brandl, mark.dickinson, r.david.murray, ron_adam
Priority: normal Keywords:

Created on 2011-01-16 15:49 by avdd, last changed 2013-10-18 23:21 by bquinlan. This issue is now closed.

Messages (24)
msg126367 - (view) Author: Adrian Dries (avdd) Date: 2011-01-16 15:49
An API such as in, e.g. futures:

   def submit(self, fn, *args, **kwargs):
       pass

cannot be used thus:

submit(foo, 1, 2, fn=bar)

I can see two options: either mangle the named parameters:

    def submit(__self, __fn, *args, **kwargs):
        pass

Or fiddle with *args:

    def submit(*args, **kwargs):
        self, fn = args[:2]
        args = args[2:]
msg126394 - (view) Author: Brian Quinlan (bquinlan) * (Python committer) Date: 2011-01-17 09:59
Arian,

This seems like such an unimportant edge case that I don't want to mess with the API just to accommodate it.

If you really need to pass an "fn" keyword argument, use:

.submit(foo, 1, 2, **{'fn': bar})
msg126395 - (view) Author: Adrian Dries (avdd) Date: 2011-01-17 11:36
What now?

Python 3.1.3 (r313:86834, Jan 17 2011, 22:33:40) 
[GCC 4.4.1] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> def foo(f, **kw):
...     pass
... 
>>> foo(1, **{'f': 2})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: foo() got multiple values for keyword argument 'f'
>>>
msg126396 - (view) Author: Brian Quinlan (bquinlan) * (Python committer) Date: 2011-01-17 11:43
Good point! I'd suggest functools.partial.
msg126410 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-01-17 16:51
Adrian's suggestions don't look to me like they fiddle with the API, but rather make the behavior match the documented API.  The existing behavior is, IMO, a very surprising corner case, especially to a less experienced Python programmer.  I do note that assertRaises in unittest has the same issue.  However, I think people are quite a bit more likely to run in to the issue with submit, since 'fn' is going to occur as a keyword argument with considerably higher frequency than 'excClass' or 'callableObj'.
msg126413 - (view) Author: Ron Adam (ron_adam) * Date: 2011-01-17 17:19
Why is this surprising?

>>> def foo(c, c=None):
...   pass
... 
  File "<stdin>", line 1
SyntaxError: duplicate argument 'c' in function definition

In the previous examples, it finds the duplicate at run time instead of compile time due to not being able to determine the contents of kwargs at compile time.  It's just a bug in your code if you do it, and it should raise an exception as it does.
msg126417 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-01-17 17:30
The reason that it is surprising is that the API is designed to allow an arbitrary function to be called, with whatever arguments and keyword arguments that function takes.  The user of the API is not necessarily going to remember that the first argument to 'submit' is named 'fn', and that therefore they must jump through special hoops if they should happen to want to submit a function that takes a keyword argument named 'fn'.  This becomes especially problematic if the function calling submit is itself accepting an arbitrary callable from a higher layer in the application.

If the first argument to submit were named something less common than 'fn', this problem might never have been noticed :)
msg126419 - (view) Author: Ron Adam (ron_adam) * Date: 2011-01-17 17:45
Is this issue referring to something in Python's library, or a hypothetical function someone may write?

If it's in the library, we can look at that case in more detail, otherwise, it's just a bad program design issue and there's nothing to do.
msg126422 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-01-17 18:06
Yes, it's about the concurrent.futures 'submit' method:

http://docs.python.org/dev/py3k/library/concurrent.futures.html#executor-objects

I've updated the title to reflect this.  There are other places in the stdlib affected by this, but they should be opened as separate issues if someone is having a problem with them.
msg126426 - (view) Author: Adrian Dries (avdd) Date: 2011-01-17 18:42
The futures case is one example of a broader API design issue.

Note also that 'self' is similarly restricted.  You might think these are 'corner cases', but to me it is poor API design.  There may well be a reasonable case for passing 'self' as a keyword arg; what right has the API to dictate otherwise?

I think Python's private name mangling provides a reasonably clean, self-documenting solution, pushing the 'corner' case further to the corner (the restriction would then be on the much-less-likely mangled names).
msg126429 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-01-17 19:33
Name mangling applies only to attributes of classes, and so does not enter in to this issue.
msg126430 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2011-01-17 19:39
def submit(*args, **kwds):

and picking out self and fn from *args might indeed be the most sensible solution here.
msg126431 - (view) Author: Adrian Dries (avdd) Date: 2011-01-17 19:41
No, private mangling applies to any identifier in class-scope:

Python 2.6.4 (r264:75706, Dec  7 2009, 18:45:15) 
[GCC 4.4.1] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> class C:
...     def f(__x): pass
... 
>>> import inspect
>>> inspect.getargspec(C.f)
ArgSpec(args=['_C__x'], varargs=None, keywords=None, defaults=None)
>>>
msg126434 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-01-17 20:24
Adrian: you are correct of course, I was misremembering and did not check the docs.  I prefer the other solution, though.
msg126449 - (view) Author: Ron Adam (ron_adam) * Date: 2011-01-18 04:02
Here is the whole method for reference...

    def submit(self, fn, *args, **kwargs):
        with self._shutdown_lock:
            if self._shutdown_thread:
                raise RuntimeError('cannot schedule new futures after shutdown')

            f = _base.Future()
            w = _WorkItem(f, fn, args, kwargs)

            self._pending_work_items[self._queue_count] = w
            self._work_ids.put(self._queue_count)
            self._queue_count += 1

            self._start_queue_management_thread()
            self._adjust_process_count()
            return f
    submit.__doc__ = _base.Executor.submit.__doc__


If self and fn are in kwargs, they are probably a *different* self and fn, than the self and fn passed to submit!
 
The current submit definition doesn't allow that, and pulling out self, and fn, would not be correct either.  

If it's still possible to change the method call signature, it should be without asterisks...

    def submit(self, fn, args, kwargs):   
        ...

Then the correct way to call it would be...

    submit(foo, [1, 2], dict(fn=bar))

There wouldn't be a conflict because the args, and keywords, (to be eventually passed to fn), are never unpacked within submit.
msg126450 - (view) Author: Ron Adam (ron_adam) * Date: 2011-01-18 04:07
Change...

   "are never unpacked within submit."

to...

    Are completely separate.   


It's the attempt to mix two function signatures together as one, that was/is the problem.
msg126455 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2011-01-18 07:14
> The current submit definition doesn't allow that, and pulling out self, and fn, would not be correct either.  

It would if they are pulled from *args.  The only difference to now is that you cannot give the self and fn args as keyword args anymore (but that is moot in the case of self, and for fn that was the aim of the issue).
msg126473 - (view) Author: Ron Adam (ron_adam) * Date: 2011-01-18 18:19
Yes, you are correct.  Pulling the first value off of args would work.

This is new for 3.2, can it still be changed?


One more thing to consider...

One of the things I look at for functions like these is, how easy is it to separate the data from the program interface?

I prefer:
    submit_data = [fn, args, kwds]
    e.submit(*submit_data)

or..
    submit_args = [(a, k), (a, k), ... ]
    for args, kwds in submit_args:
        e.submit(fn, args, kwds)

But its a trade off against easier direct calling.  My feelings, is submit will be used more in an indirect way, like these examples, rather than being used directly by writing out each submit separately.
msg126482 - (view) Author: Adrian Dries (avdd) Date: 2011-01-18 19:28
Have your cake and eat it:

def submit(self, fn, args, kw):
    # submit implementation

def sugar(*args, **kw):
    return args[0].submit(args[1], args[2:], kw)
msg126486 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2011-01-18 19:39
It could be changed, as it is not an incompatible API change.  However, I'd like Brian to ok it.
msg127712 - (view) Author: Brian Quinlan (bquinlan) * (Python committer) Date: 2011-02-01 23:57
Sorry for taking so long to reply - I was on holidays until today.

This is an incompatible API change (since people may be providing "fn" by keyword) so we should probably hold off until 3.3.

I also don't really like that the signature for submit will become less readable. And the fix is pretty trivial i.e. functools.partial.

So really I'm -0 on this. But if anyone cares enough, I'll happily review and apply patches that change the submit signature to the one that Adrian proposed.

def sugar(*args, **kw):
    return args[0].submit(args[1], args[2:], kw)
msg127722 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2011-02-02 08:16
Sure, a change to 3.2 is out of the question anyway right now.
msg194540 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2013-08-06 13:09
#15966 is related.  I suggest closing this issue as "won't fix", too.
msg200329 - (view) Author: Brian Quinlan (bquinlan) * (Python committer) Date: 2013-10-18 23:21
I think that we are happy to not fix this.
History
Date User Action Args
2013-10-18 23:21:16bquinlansetstatus: open -> closed
resolution: accepted -> wont fix
messages: + msg200329
2013-08-06 13:28:47mark.dickinsonlinkissue15966 superseder
2013-08-06 13:09:28mark.dickinsonsetnosy: + mark.dickinson
messages: + msg194540
2011-02-02 08:16:28georg.brandlsetnosy: georg.brandl, bquinlan, ron_adam, r.david.murray, avdd
messages: + msg127722
2011-02-01 23:57:24bquinlansetstatus: closed -> open

messages: + msg127712
resolution: wont fix -> accepted
nosy: georg.brandl, bquinlan, ron_adam, r.david.murray, avdd
2011-01-18 19:39:01georg.brandlsetnosy: georg.brandl, bquinlan, ron_adam, r.david.murray, avdd
messages: + msg126486
2011-01-18 19:28:43avddsetnosy: georg.brandl, bquinlan, ron_adam, r.david.murray, avdd
messages: + msg126482
2011-01-18 18:19:16ron_adamsetnosy: georg.brandl, bquinlan, ron_adam, r.david.murray, avdd
messages: + msg126473
2011-01-18 07:14:25georg.brandlsetnosy: georg.brandl, bquinlan, ron_adam, r.david.murray, avdd
messages: + msg126455
2011-01-18 04:07:52ron_adamsetnosy: georg.brandl, bquinlan, ron_adam, r.david.murray, avdd
messages: + msg126450
2011-01-18 04:02:33ron_adamsetnosy: georg.brandl, bquinlan, ron_adam, r.david.murray, avdd
messages: + msg126449
2011-01-17 20:24:31r.david.murraysetnosy: georg.brandl, bquinlan, ron_adam, r.david.murray, avdd
messages: + msg126434
2011-01-17 19:41:25avddsetnosy: georg.brandl, bquinlan, ron_adam, r.david.murray, avdd
messages: + msg126431
2011-01-17 19:39:16georg.brandlsetnosy: + georg.brandl
messages: + msg126430
2011-01-17 19:33:34r.david.murraysetnosy: bquinlan, ron_adam, r.david.murray, avdd
messages: + msg126429
2011-01-17 18:42:27avddsetnosy: bquinlan, ron_adam, r.david.murray, avdd
messages: + msg126426
2011-01-17 18:06:49r.david.murraysetnosy: bquinlan, ron_adam, r.david.murray, avdd
messages: + msg126422
title: **kwargs unnecessarily restricted in API -> **kwargs unnecessarily restricted in concurrent.futures 'submit' API
2011-01-17 17:45:55ron_adamsetnosy: bquinlan, ron_adam, r.david.murray, avdd
messages: + msg126419
2011-01-17 17:30:36r.david.murraysetnosy: bquinlan, ron_adam, r.david.murray, avdd
messages: + msg126417
2011-01-17 17:19:25ron_adamsetnosy: + ron_adam
messages: + msg126413
2011-01-17 16:51:46r.david.murraysetnosy: + r.david.murray
messages: + msg126410
2011-01-17 11:43:41bquinlansetmessages: + msg126396
2011-01-17 11:36:41avddsetmessages: + msg126395
2011-01-17 09:59:30bquinlansetstatus: open -> closed

messages: + msg126394
resolution: wont fix
2011-01-16 16:04:55georg.brandlsetassignee: bquinlan

nosy: + bquinlan
versions: + Python 3.3, - Python 3.2
2011-01-16 15:49:46avddcreate