[Python-Dev] A Desperate Plea For Introspection (aka: BDFAP Needed)

[Guido van Rossum]

On Fri, Jun 22, 2012 at 12:10 PM, Yury Selivanov
<yselivanov.ml at gmail.com> wrote:
> Guido,
>
> On 2012-06-22, at 2:52 PM, Guido van Rossum wrote:
>
>> This looks great, much better than the version I reviewed half a year
>> ago! Thanks you and others (especially Yuri) for all your efforts in
>> guiding the discussion and implementing as the discussion went along;
>> also thanks to Nick for participating to intensely.
>>
>> Quick review notes:
>>
>> (1) I don't like APIs that require you to use hasattr(), which you use
>> for return_annotation. Since you have Signature.empty to remove it,
>> why not set it to Signature.empty when not set? Ditto for other values
>> that are currently not set when they don't apply (Parameter.empty I
>> think.)
>
> I think that if a function lacks an annotation, that should be reflected
> in the same way for its signature.
>
> Currently:
>
>    if hasattr(signature, 'return_annotation'):
>
> If we use Signature.empty:
>
>    if signature.return_annotation is not signature.empty:
>
> So (in my humble opinion) it doesn't simplify things too much.

But it is much more of a pain if someone just wants to pass the value
on or print it. Instead of

  print(sig.return_annotation)

you'd have to write

  if hasattr(sig, 'return_annotation'):
    print(sig.return_annotation)
  else:
    print('empty')

In general hasattr() (or getattr() with a 3rd arg) has a code smell;
making it a required part of an API truly stinks IMO.

> And also you can use 'try .. except AttributeError .. else' blocks,
> which make code even more readable.

No, try/except blocks make code *less* readable.

> All in all, I don't have a very strong opinion in this topic.

But I do. :-)

> 'empty' will also work.  When python-dev collectively decided to
> go with missing attributes, 'empty' didn't yet exist (we added
> it with 'replace()' methods).
>
> If you think that using 'empty' is better, we can add that to the PEP.

Yes, please do.

>> (2) Could use an example on how to remove and add parameters using replace().
>
> You have to build a new list of parameters and then pass it to 'replace()'.
> Example (from the actual signature() function implementation):
>
>    if isinstance(obj, types.MethodType):
>        # In this case we skip the first parameter of the underlying
>        # function (usually `self` or `cls`).
>        sig = signature(obj.__func__)
>        return sig.replace(parameters=tuple(sig.parameters.values())[1:])
>
>> (3) You are using name(arg1, *, arg2) a lot. I think in most cases you
>> mean for arg2 to be an optional keyword arg, but this notation doesn't
>> convey that it is optional. Can you clarify?
>
> Yes, I meant optional.  Would 'name(arg1, *, [arg2])' be better?

Hardly, because that's not valid syntax. I'd write name(arg1, *,
arg2=<default>).

>> (4) "If the object is a method" -- shouldn't that be "bound method"?
>> (Unbound methods are undetectable.) Or is there some wider definition
>> of method? What does it do for static or class methods?
>
> Yes, it should be "If the object is a bound method".  We'll fix this
> shortly.

Great.

> classmethod as a descriptor returns a BoundMethod (bound to the class),
> staticmethod returns the original unmodified function, so both of
> them are supported automatically.

Oh, great. IIRC it wasn't always like that. Maybe just add this to the
PEP as a note?

>> (5) Too bad there's no proposal for adding signatures to builtin
>> functions/methods, but understood.
>>
>> Of these, only (1) is a blocker for PEP acceptance -- I'd either like
>> to see this defended vigorously (maybe it was discussed? then please
>> quote, I can't go back and read the threads) or changed.
>>
>> Otherwise it looks great!
>>
>> --Guido
>
> Thanks!

Thank *you* for your hard work!


-- 
--Guido van Rossum (python.org/~guido)