[Python-Dev] Review of PEP 362 (signature object) (original) (raw)
Guido van Rossum guido at python.org
Tue Mar 13 01:51:13 CET 2012
- Previous message: [Python-Dev] PEP 335 officially rejected
- Next message: [Python-Dev] Review of PEP 362 (signature object)
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
I'm very sympathetic to this PEP. I would accept it outright except I have a few quibbles on details, and some questions and remarks.
There are several examples of poor English grammar, perhaps from your co-author. Can you fix these? (Do you need me to produce a list?)
You're using an informal notation to indicate compound types, e.g. dict(str, object). I'm not sure it's worth using this particular notation without defining it (although maybe the time is ripe for creating a PEP that proposes a standard use for parameter annotations...). You're also not using it very consistently
sig.name is currently the unqualified function name. Now we have qualname maybe this should be the qualified name instead?
Did you think about whether var_args and var_kw_args should be '' or None or undefined if these aren't present in the actual definition?
If there is no return annotation, is return_annotation None or undefined? (TBH I think undefined is awkward because you'd have to use hasattr() to test for its presence. I'd be okay with equating None with no return annotation. For parameter annotations I'm less sure, it's not so bad to test for presence in the dict, and you can easily use .get().)
I don't quite understand how bind() is supposed to work. Maybe an example would help? (It could also use some motivation. I think this is meant to expose a canonical version of the algorithm that maps arguments to parameters. What's a use case?)
Why is bind() listed under attributes, while there's also a list of methods? Is it something funky about self?
The PEP still seems to support tuple unpacking, which is no longer supported in Python 3. Please take it out.
I see it was my idea to give kw-only parameter a valid but meaningless position. I think I want to revert my opinion; it would be odd if there's a (kw-only) parameter 5 that cannot correspond to argument 5. So let's set it to None if it's kw-only. Maybe sig.parameters should not have these guys? Or it should have a separate sig.kw_only_parameters which is a dict??????
There is mention of has_annotation but no definition. Is this due to a half revision of the PEP? I sort of see the point but maybe it's more pragmatic to set it to None for an absent annotation? (Later: maybe I see, there's a similar pattern for defaults, and it does make sense to distinguish between "x" and "x = y".)
And why are there two ways of getting the annotations, one via sig.var_annotations[v] and once via sig[v].annotation ?
Actually I now see there are also, kind of, two ways to access the Parameter objects: sig[v] and sig.parameters[i]. But maybe that's more defensible since we want to be able to access them by position or by name.
You have some examples like "var_args(*[1,2,3])" -- I think that should just be "var_args(1, 2, 3)" right? Similar "var_kw_args(**{'a': 1})" should be "var_kw_args(a=1)"...
You have some example code using try: [] // except KeyError: pass // else:
. Wouldn't that be expressed cleaner using if in : ?Similar, this smells a bit; can you explain or improve? try: duck = sig.var_annotations[sig.var_kw_args] except (KeyError, AttributeError):
That's all I have for now; on to reject^Wreview some more PEPs...
-- --Guido van Rossum (python.org/~guido)
- Previous message: [Python-Dev] PEP 335 officially rejected
- Next message: [Python-Dev] Review of PEP 362 (signature object)
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]