python / cpython

The Python programming language
https://www.python.org
Other
62.28k stars 29.92k forks source link

inspect.Signature doesn't support user classes without __init__ or __new__ #64507

Closed larryhastings closed 10 years ago

larryhastings commented 10 years ago
BPO 20308
Nosy @brettcannon, @ncoghlan, @larryhastings, @skrah, @1st1
Files
  • signature_plain_cls_01.patch
  • signature_plain_cls_02.patch
  • signature_plain_cls_04.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: ```python assignee = None closed_at = created_at = labels = ['type-bug', 'library'] title = "inspect.Signature doesn't support user classes without __init__ or __new__" updated_at = user = 'https://github.com/larryhastings' ``` bugs.python.org fields: ```python activity = actor = 'larry' assignee = 'none' closed = True closed_date = closer = 'yselivanov' components = ['Library (Lib)'] creation = creator = 'larry' dependencies = [] files = ['33556', '33557', '33756'] hgrepos = [] issue_num = 20308 keywords = ['patch'] message_count = 17.0 messages = ['208502', '208504', '208507', '208508', '208510', '208511', '208512', '208513', '208514', '208515', '208516', '209481', '209489', '209490', '209491', '209522', '210098'] nosy_count = 6.0 nosy_names = ['brett.cannon', 'ncoghlan', 'larry', 'skrah', 'python-dev', 'yselivanov'] pr_nums = [] priority = 'normal' resolution = 'fixed' stage = 'needs patch' status = 'closed' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue20308' versions = ['Python 3.4'] ```

    larryhastings commented 10 years ago

    Boy was I surprised by this:

    >>> class C: pass
    ... 
    >>> import inspect
    >>> inspect.signature(C)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/home/larry/src/python/clinic_c_types_hacking/Lib/inspect.py", line 1557, in signature
        raise ValueError('callable {!r} is not supported by signature'.format(obj))
    ValueError: callable <class '__main__.C'> is not supported by signature

    Why not? C is a callable object. If you can't find a __new or an __init in the class, its signature should be pretty predictable.

    (It returns a signature if you add either a __new, an __init, or both.)

    1st1 commented 10 years ago

    Well, the current code looks for __init or __new. The only ones it can find is the 'object.__init__' which are blacklisted, because they are in C.

    The question is do (or will) 'object.__new' or '__init' have a signature defined with the clinic?

    larryhastings commented 10 years ago

    __new and __init methods are very special. They can't have signatures, because the mechanism we use to store the signatures won't work. (We hide them as a special first line of the docstring, and __new and __init can't have custom docstrings.)

    In my next patch for bpo-20189,

    inspect.signature(object)

    will return a valid signature object. But

    class C: pass
    inspect.signature(C)

    still fails in that branch.

    1st1 commented 10 years ago

    In this case it would probably be best to just special case classes that don't have __init or __new defined to return an empty signature without parameters.

    I can also make a special case for object.__init and object.__new functions, if someone would want to introspect them directly.

    larryhastings commented 10 years ago

    If we need a special case for user classes without __new or __init, then do that. But I wouldn't say we should special case object.__new and object.__init.

    1st1 commented 10 years ago

    Please take a look at the attached patch (signature_plain_cls_01.patch)

    Now, the patch addresses two kind of classes:

    class C: pass
    and 
    class C(type): pass

    For metaclasses, signature will return a signature with three positional-only parameters - (name, bases, dct). [let's discuss this]

    The patch doesn't address 'object' or 'type' objects directly, though, so 'signature(object)' and 'signature(type)' are still a ValueError.

    larryhastings commented 10 years ago

    inspect.signature(object) works fine in my (not yet posted) latest bpo-20189 patch.

    inspect.signature(type) doesn't work, because it's not clear what the signature for type should be. There's the one-argument and three-argument approaches. This is technically true:

    "(object_or_name, [bases, dict])"

    However, a) it's painful to look at, b) I can't communicate the idea of an "optional group" in an inspect.Signature object right now (although I guess we're going to hash that out somewhere, which is good).

    If we can agree on a good signature for inspect.signature(type), I can make it happen.

    1st1 commented 10 years ago

    Couple of thoughts:

    1. "(object_or_name, [bases, dict])" is a signature for the "type" function, and yes, on that we need to agree how it looks like. Maybe exactly as you proposed, as it is what it is after all.

    2. For user-defined metaclasses without __init or __new (such as "class C(type)"), we can just return, IMO, "(name, bases, dict)", as, although, it is possible to call "C" with only one argument, it's hardly a good practice, and I doubt it very much that someone does such things.

    larryhastings commented 10 years ago

    Special cases aren't special enough to break the rules. If the signature of a metaclass is "(object_or_name, [bases, dict])", then we must not special-case it to pretend that "(object)" works. I agree it's a bad idea to actually *do* that, but there are best-practice principles at stake here.

    1st1 commented 10 years ago

    When in doubt, import this ;)

    Agree. So the best course would be: make a patch for plain classes (not metaclasses). Fix signatures for metaclasses without __init/new__ when we have groups support for parameters, so that we can have (obj_or_name, [bases, dict]) kind of signatures.

    1st1 commented 10 years ago

    Attached is a stripped down patch that special-cases classes without __init/new__. Not metaclasses, for that we can start a new issue.

    1st1 commented 10 years ago

    Larry,

    Please take a look at the attached 'signature_plain_cls_04.patch'. This one supports classes (returns signature of 'object' builtin), and metaclasses (returns signature of 'type' builtin).

    larryhastings commented 10 years ago

    Looks perfect! Please check it in.

    larryhastings commented 10 years ago

    (With a Misc/NEWS entry of course.)

    1762cc99-3127-4a62-9baf-30c3d0f51ef7 commented 10 years ago

    New changeset b9ca2019bcb9 by Yury Selivanov in branch 'default': inspect.signature: Support classes without user-defined __init/new__ bpo-20308 http://hg.python.org/cpython/rev/b9ca2019bcb9

    5531d0d8-2a9c-46ba-8b8b-ef76132a492c commented 10 years ago

    One test fails --without-doc-strings:

    http://buildbot.python.org/all/builders/AMD64%20FreeBSD%209.0%203.x/builds/6266/steps/test/logs/stdio

    larryhastings commented 10 years ago

    That buildbot is happy now. Thanks for pointing it out!