inspect.signature: inaccuracies for partial functions

[1st1]

BPO	21117
Nosy	@brettcannon, @ncoghlan, @larryhastings, @bitdancer, @1st1
Files	signature_partial_fix_01.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/1st1'
closed_at = <Date 2014-04-08.15:32:44.058>
created_at = <Date 2014-03-31.20:30:36.388>
labels = ['type-bug', 'library']
title = 'inspect.signature: inaccuracies for partial functions'
updated_at = <Date 2014-04-08.15:32:44.057>
user = 'https://github.com/1st1'

bugs.python.org fields:

activity = <Date 2014-04-08.15:32:44.057>
actor = 'yselivanov'
assignee = 'yselivanov'
closed = True
closed_date = <Date 2014-04-08.15:32:44.058>
closer = 'yselivanov'
components = ['Library (Lib)']
creation = <Date 2014-03-31.20:30:36.388>
creator = 'yselivanov'
dependencies = []
files = ['34706']
hgrepos = []
issue_num = 21117
keywords = ['patch']
message_count = 23.0
messages = ['215265', '215269', '215270', '215271', '215276', '215277', '215286', '215290', '215295', '215317', '215318', '215323', '215324', '215325', '215327', '215328', '215329', '215340', '215341', '215347', '215402', '215527', '215767']
nosy_count = 6.0
nosy_names = ['brett.cannon', 'ncoghlan', 'larry', 'r.david.murray', 'python-dev', 'yselivanov']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'needs patch'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue21117'
versions = ['Python 3.4', 'Python 3.5']

[1st1]

There is a small detail in the current Signature class implementation, in regards to how partial signatures are treated. Consider the following example:

   def foo(a, b): pass
   foo_partial = functools.partial(foo, 'spam')

Now, the signature of 'foo_partial' is '(a="spam", b)', which (strictly speaking) is not a valid python function signature. For cases like that, we have a private "Parameter._partial_kwarg" attribute. When this attribute is set to True, it means, that this parameter's default came from partial (or alike) function. Parameter instances with '_partial_kwarg=True' are treated a bit differently during signature validation and binding.

A small and nasty detail: Parameter.__eq__ ignores value of '_partial_kwarg'. Because, for the following code:

   def bar(a, b=10): pass
   def baz(a, b): pass
   baz2 = functools.partial(baz, b=10)

signature of 'bar' is equal to 'baz2'.

In light of making signatures hashable, the obvious question was raised: should __hash__ account for '_partial_kwarg' value or not. I think it should. But in this case, it should be really obvious, if parameter was modified by partial or not.

Hence, I propose to add two more classes:

• PartialSignature(Signature)
• PartialParameter(Parameter)

Results of signature(functools.partial(..)) and Signature.bind_partial(..) will be instances of PartialSignature. It will be OK if some PartialSignature is equal to Signature, but they will have different __hash__es.

What do you think?

[bitdancer]

I believe it is a python invariant that a == b implies hash(a) == hash(b). I don't see why the two signatures should be equal. I'm not even sure why the bound argument shows up in the signature of the partial. That surprises me.

[ncoghlan]

The already bound arguments should be treated as additional keyword-only
arguments, and already bound positional arguments hidden completely.

I'm +0 on new types to clean that up if necessary, but would prefer it if
we could just improve the translation to ordinary signature objects instead.

[ncoghlan]

On 1 Apr 2014 07:36, "Nick Coghlan" <report@bugs.python.org> wrote:

Nick Coghlan added the comment:

The already bound arguments should be treated as additional keyword-only
arguments, and already bound positional arguments hidden completely.

Oops: already bound positional-only arguments should be hidden.

[1st1]

@nick:

Oops: already bound positional-only arguments should be hidden.
Hm, good catch. I'm not sure we currently do this. I'll check if this needs to be fixed (in 3.4.1 too).

I'm +0 on new types to clean that up if necessary, but would prefer it if
we could just improve the translation to ordinary signature objects instead.

I'm not sure I understand what you mean by "translation to ordinary signature objects". Could you please elaborate on this?

@r. David:

I believe it is a python invariant that a == b implies hash(a) == hash(b).

I think that 'hash(a) == hash(b)' means that 'a == b' (strongly). But not reverse. Or am I wrong?

I don't see why the two signatures should be equal. I'm not even sure why the bound argument shows up in the signature of the partial.

Well, why shouldn't they be equal? They have same parameters, same default values. Quoting PEP-362: """...two signatures are equal only when their corresponding parameters are equal and have the exact same names...""". Moreover, this behaviour is implemented since 3.3.

But their hashes shouldn't be equal, that's something I can agree on.

[ncoghlan]

On 1 Apr 2014 08:17, "Yury Selivanov" <report@bugs.python.org> wrote:

> I'm +0 on new types to clean that up if necessary, but would prefer it
if
we could just improve the translation to ordinary signature objects
instead.

I'm not sure I understand what you mean by "translation to ordinary
signature objects". Could you please elaborate on this?

If arguments bound by position disappear from the signature, and those
bound by keyword are mapped to keyword-only parameters with a default, we
should get a valid and accurate signature. That's a 3.4.1 bug fix, rather
than a 3.5 feature.

[1st1]

@nick:

Agreed on positional-only stuff.

If arguments bound by position disappear from the signature, and those
bound by keyword are mapped to keyword-only parameters with a default, we
should get a valid and accurate signature.

But what about my example from the first message:

   def foo(a, b): pass
   foo_partial = functools.partial(foo, 'spam')

'foo_partial' now has the following signature: (a='spam', b); where 'a' is a positional_or_keyword parameter, i.e. you can still do 'foo_partial(10, 20)', or 'foo_partial(b=20, a=10)'.

(a='spam', b) is not a valid python pure function signature, but is a perfectly valid signature of partial function. And since its arguments aren't positional-only, we shouldn't hide anything.

That's why I have this private '_partial_kwarg' attribute, which I don't like and want to remove by adding a PartialParameter subclass. Otherwise, we have something that is hidden and non-documented, but affects Parameter.__hash__ and some other logic.

[1st1]

@nick: oh, it took me some time to realize that your suggestion to transform positional-or-keyword to keyword-only parameters is correct. If we do this, we no longer need '_partial_kwarg' hack. I'll work on the patch. Thank you.

[ncoghlan]

I believe Yury already figured out what I meant, but to make it entirely clear, after the change, this example:

   def foo(a, b): pass
   foo_partial = functools.partial(foo, 'spam')
   foo_partial2 = functools.partial(foo, a='spam')

Should lead to the following signature for both "foo_partial" and "foo_partial2":

(b, *, a='spam')

That accurately indicates that "a" is now effectively a keyword only parameter - the first supplied positional argument will map to "b", and attempting to supply *two* positional arguments will fail.

Correctly handing *args may get a little interesting, but should be feasible.