Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inheriting from class that defines __new__ causes inspect.signature to always return (*args, **kwargs) for constructor #85074

Closed
ezyang mannequin opened this issue Jun 7, 2020 · 17 comments
Labels
3.9 only security fixes 3.10 only security fixes 3.11 bug and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@ezyang
Copy link
Mannequin

ezyang mannequin commented Jun 7, 2020

BPO 40897
Nosy @gvanrossum, @ambv, @ezyang, @serhiy-storchaka, @1st1, @jonathanslenders, @hongweipeng, @miss-islington, @mauvilsa, @FFY00
PRs
  • bpo-40897:Give priority to using the current class constructor in inspect.signature #23336
  • bpo-40897:Give priority to using the current class constructor in inspect.signature #27177
  • [3.10] bpo-40897:Give priority to using the current class constructor in inspect.signature (GH-27177) #27189
  • [3.9] bpo-40897: Partially backport GH-22583's refactor of inspect.py to allow bugfix backports #27193
  • [3.9] bpo-40897:Give priority to using the current class constructor in inspect.signature (GH-27177) #27209
  • 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 = None
    closed_at = <Date 2021-07-17.08:37:04.648>
    created_at = <Date 2020-06-07.03:37:38.227>
    labels = ['type-bug', 'library', '3.9', '3.10', '3.11']
    title = 'Inheriting from class that defines __new__ causes inspect.signature to always return (*args, **kwargs) for constructor'
    updated_at = <Date 2021-07-17.08:37:04.647>
    user = 'https://github.com/ezyang'

    bugs.python.org fields:

    activity = <Date 2021-07-17.08:37:04.647>
    actor = 'lukasz.langa'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-07-17.08:37:04.648>
    closer = 'lukasz.langa'
    components = ['Library (Lib)']
    creation = <Date 2020-06-07.03:37:38.227>
    creator = 'ezyang'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 40897
    keywords = ['patch']
    message_count = 17.0
    messages = ['370870', '370888', '370958', '370970', '371023', '387139', '387184', '397248', '397470', '397606', '397617', '397619', '397623', '397633', '397705', '397708', '397709']
    nosy_count = 11.0
    nosy_names = ['gvanrossum', 'lukasz.langa', 'ezyang', 'serhiy.storchaka', 'yselivanov', 'ralf.gommers', 'jonathan.slenders', 'hongweipeng', 'miss-islington', 'mauvilsa', 'FFY00']
    pr_nums = ['23336', '27177', '27189', '27193', '27209']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue40897'
    versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

    @ezyang
    Copy link
    Mannequin Author

    ezyang mannequin commented Jun 7, 2020

    Consider the following program:

    import inspect
    from typing import Generic, TypeVar
    
    T = TypeVar('T')
    
    class A(Generic[T]):
        def __init__(self) -> None:
            pass
    
    print(inspect.signature(A))
    

    I expect inspect.signature to return () as the signature of the constructor of this function. However, I get this:

    $ python3 foo.py
    (*args, **kwds)
    

    Although it is true that one cannot generally rely on inspect.signature to always give the most accurate signature (because there may always be decorator or metaclass shenanigans getting in the way), in this particular case it seems especially undesirable because Python type annotations are supposed to be erased at runtime, and yet here inheriting from Generic (simply to add type annotations) causes a very clear change in runtime behavior.

    @ezyang ezyang mannequin added 3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Jun 7, 2020
    @serhiy-storchaka
    Copy link
    Member

    It is not special for Generic, but happens with every type implementing __new__.

    class A:
        def __new__(cls, a=1, *args, **kwargs):
            return object.__new__(cls)
    
    class B(A):
        def __init__(self, b):
            pass
    
    import inspect
    print(inspect.signature(B))

    The above example prints "(a=1, *args, **kwargs)" instead of "(b)".

    @gvanrossum
    Copy link
    Member

    So does that make this "not a bug"? Or is there something to document? For technical reasons we can't just add a __init__ method to Generic, and I doubt that it's feasible to change inspect.signature().

    @serhiy-storchaka
    Copy link
    Member

    I think that inspect.signature() could be made more smart. It should take into account signatures of both __new__ and __init__ and return the most specific compatible signature.

    @gvanrossum
    Copy link
    Member

    Changing the topic to not point fingers at Generic.

    @gvanrossum gvanrossum changed the title Inheriting from Generic causes inspect.signature to always return (*args, **kwargs) for constructor (and all subclasses) Inheriting from class that defines __new__ causes inspect.signature to always return (*args, **kwargs) for constructor Jun 8, 2020
    @gvanrossum gvanrossum changed the title Inheriting from Generic causes inspect.signature to always return (*args, **kwargs) for constructor (and all subclasses) Inheriting from class that defines __new__ causes inspect.signature to always return (*args, **kwargs) for constructor Jun 8, 2020
    @JonathanSlenders
    Copy link
    Mannequin

    JonathanSlenders mannequin commented Feb 17, 2021

    The following patch to inspect.py solves the issue that inspect.signature() returns the wrong signature on classes that inherit from Generic. Not 100% sure though if this implementation is the cleanest way possible. I've been looking into attaching a __wrapped__ to Generic as well, without success. I'm not very familiar with the inspect code.

    To me, this fix is pretty important. ptpython, a Python REPL, has the ability to show the function signature of what the user is currently typing, and with codebases that have lots of generics, there's nothing really useful we can show.

     $ diff inspect.old.py  inspect.py  -p
    *** inspect.old.py      2021-02-17 11:35:50.787234264 +0100
    --- inspect.py  2021-02-17 11:35:10.131407202 +0100
    *************** import sys
    *** 44,49 ****
    --- 44,50 

      import tokenize
      import token
      import types
    + import typing
      import warnings
      import functools
      import builtins
    *************** def _signature_get_user_defined_method(c
    *** 1715,1720 ****
    --- 1716,1725 ----
          except AttributeError:
              return
          else:
    +         if meth in (typing.Generic.__new__, typing.Protocol.__new__):
    +             # Exclude methods from the typing module.
    +             return
    +
              if not isinstance(meth, _NonUserDefinedCallables):
                  # Once '__signature__' will be added to 'C'-level
                  # callables, this check won't be necessary

    For those interested, the following monkey-patch has the same effect:

    def monkey_patch_typing() -> None:
        import inspect, typing
        def _signature_get_user_defined_method(cls, method_name):
            try:
                meth = getattr(cls, method_name)
            except AttributeError:
                return
            else:
                if meth in (typing.Generic.__new__, typing.Protocol.__new__):
                    # Exclude methods from the typing module.
                    return
    
                if not isinstance(meth, inspect._NonUserDefinedCallables):
                    # Once '__signature__' will be added to 'C'-level
                    # callables, this check won't be necessary
                    return meth
    
        inspect._signature_get_user_defined_method = _signature_get_user_defined_method
    
    monkey_patch_typing()

    @gvanrossum
    Copy link
    Member

    I doubt that solution is correct, given that we already established that the problem is *not* specific to Generic.

    @mauvilsa
    Copy link
    Mannequin

    mauvilsa mannequin commented Jul 10, 2021

    I think this is affecting me or it is a new similar issue. And it is not only for python 3.9, but also from 3.6 up. I am working on making code configurable based on signatures (see https://jsonargparse.readthedocs.io/en/stable/#classes-methods-and-functions). Now we need this to work for datetime.timedelta which defines parameters in __new__ instead of __init__. The following happens:

    >>> from datetime import timedelta
    >>> import inspect
    >>> inspect.signature(timedelta.__new__)
    <Signature (*args, **kwargs)>
    >>> inspect.signature(timedelta.__init__)
    <Signature (self, /, *args, **kwargs)>
    >>> inspect.signature(timedelta)
    ...
    ValueError: no signature found for builtin type <class 'datetime.timedelta'>

    I am expecting to get parameters for days, seconds, microseconds, milliseconds, minutes, hours and weeks, see

    cpython/Lib/datetime.py

    Lines 461 to 462 in bfe544d

    def __new__(cls, days=0, seconds=0, microseconds=0,
    milliseconds=0, minutes=0, hours=0, weeks=0):
    .

    Hopefully this gives some insight into what should be done. Independent from the fix, I would like to know if currently there is any way I can get the signature for __new__ that I could use until there is a proper fix for it.

    @mauvilsa
    Copy link
    Mannequin

    mauvilsa mannequin commented Jul 14, 2021

    I created another issue since the problem appears to be a bit different: https://bugs.python.org/issue44618

    @hongweipeng
    Copy link
    Mannequin

    hongweipeng mannequin commented Jul 16, 2021

    >>> from datetime import timedelta as a
    >>> from _datetime import timedelta as b
    >>> a is b
    True
    >>>

    timedelta is a C-level class, so inspect.signature(timedelta) is the same with inspect.signature(int).

    But signature allow C-level function such as inspect.signature(len), I think one way to improve is to use the C-level function when the python-level function cannot be found.

    @ambv
    Copy link
    Contributor

    ambv commented Jul 16, 2021

    New changeset 6aab5f9 by Weipeng Hong in branch 'main':
    bpo-40897:Give priority to using the current class constructor in inspect.signature (bpo-27177)
    6aab5f9

    @ambv
    Copy link
    Contributor

    ambv commented Jul 16, 2021

    We won't be backporting this fix to 3.9 due to larger changes between versions.

    @ambv ambv added 3.10 only security fixes 3.11 bug and security fixes labels Jul 16, 2021
    @ambv
    Copy link
    Contributor

    ambv commented Jul 16, 2021

    New changeset 948e39a by Miss Islington (bot) in branch '3.10':
    bpo-40897:Give priority to using the current class constructor in inspect.signature (GH-27177) (bpo-27189)
    948e39a

    @ambv
    Copy link
    Contributor

    ambv commented Jul 16, 2021

    On second thought it's a bummer not to fix this in 3.9.x that will still be the only stable version until October. I'll refactor the relevant part of inspect.py in 3.9 to make the backport applicable.

    @ambv
    Copy link
    Contributor

    ambv commented Jul 17, 2021

    New changeset ed2db9b by Łukasz Langa in branch '3.9':
    bpo-40897: Partially backport #66773's refactor of inspect.py to allow bugfix backports (bpo-27193)
    ed2db9b

    @ambv
    Copy link
    Contributor

    ambv commented Jul 17, 2021

    New changeset df7c629 by Miss Islington (bot) in branch '3.9':
    bpo-40897:Give priority to using the current class constructor in inspect.signature (GH-27177) (GH-27209)
    df7c629

    @ambv
    Copy link
    Contributor

    ambv commented Jul 17, 2021

    Thanks! ✨ 🍰 ✨

    @ambv ambv closed this as completed Jul 17, 2021
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes 3.10 only security fixes 3.11 bug and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants