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

functools: singledispatchmethod doesn't work with classmethod #83860

Closed
vr2262 mannequin opened this issue Feb 18, 2020 · 17 comments
Closed

functools: singledispatchmethod doesn't work with classmethod #83860

vr2262 mannequin opened this issue Feb 18, 2020 · 17 comments
Labels
3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@vr2262
Copy link
Mannequin

vr2262 mannequin commented Feb 18, 2020

BPO 39679
Nosy @ncoghlan, @glyph, @ambv, @mgrandi, @ilevkivskyi, @miss-islington, @tirkarthi, @mental32, @vr2262, @FFY00, @EugenePY, @AlexWaygood
PRs
  • bpo-39679: Add tests for classmethod/staticmethod singledispatchmethods #29034
  • [3.10] bpo-39679: Add tests for classmethod/staticmethod singledispatchmethods (GH-29034) #29072
  • [3.9] bpo-39679: Fix singledispatchmethod classmethod/staticmethod bug #29087
  • 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-10-28.16:03:07.339>
    created_at = <Date 2020-02-18.19:16:15.217>
    labels = ['type-bug', 'library', '3.9']
    title = "functools: singledispatchmethod doesn't work with classmethod"
    updated_at = <Date 2021-10-28.16:03:07.338>
    user = 'https://github.com/vr2262'

    bugs.python.org fields:

    activity = <Date 2021-10-28.16:03:07.338>
    actor = 'lukasz.langa'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-10-28.16:03:07.339>
    closer = 'lukasz.langa'
    components = ['Library (Lib)']
    creation = <Date 2020-02-18.19:16:15.217>
    creator = 'Viktor Roytman'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 39679
    keywords = ['patch']
    message_count = 15.0
    messages = ['362233', '362253', '362434', '362442', '371230', '372405', '378344', '380221', '395910', '404147', '404348', '404362', '404363', '405195', '405196']
    nosy_count = 13.0
    nosy_names = ['ncoghlan', 'glyph', 'lukasz.langa', 'markgrandi', 'levkivskyi', 'miss-islington', 'xtreak', 'mental', 'Viktor Roytman', 'FFY00', 'EugenePY', 'dmkulazhenko', 'AlexWaygood']
    pr_nums = ['29034', '29072', '29087']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue39679'
    versions = ['Python 3.9']

    @vr2262
    Copy link
    Mannequin Author

    vr2262 mannequin commented Feb 18, 2020

    I couldn't get the example given for the interaction between @singledispatchmethod and @classmethod to work https://docs.python.org/3/library/functools.html?highlight=singledispatch#functools.singledispatchmethod

        from functools import singledispatchmethod
        
        
        class Negator:
            @singledispatchmethod
            @classmethod
            def neg(cls, arg):
                raise NotImplementedError("Cannot negate a")
        
            @neg.register
            @classmethod
            def _(cls, arg: int):
                return -arg
        
            @neg.register
            @classmethod
            def _(cls, arg: bool):
                return not arg
        
        
        if __name__ == "__main__":
            print(Negator.neg(0))
            print(Negator.neg(False))

    Leads to

        $ python -m bad_classmethod_as_documented
        Traceback (most recent call last):
          File "/usr/lib/python3.8/runpy.py", line 193, in _run_module_as_main
            return _run_code(code, main_globals, None,
          File "/usr/lib/python3.8/runpy.py", line 86, in _run_code
            exec(code, run_globals)
          File "/home/viktor/scratch/bad_classmethod_as_documented.py", line 4, in <module>
            class Negator:
          File "/home/viktor/scratch/bad_classmethod_as_documented.py", line 12, in Negator
            def _(cls, arg: int):
          File "/usr/lib/python3.8/functools.py", line 906, in register
            return self.dispatcher.register(cls, func=method)
          File "/usr/lib/python3.8/functools.py", line 848, in register
            raise TypeError(
        TypeError: Invalid first argument to `register()`: <classmethod object at 0x7f37d1469070>. Use either `@register(some_class)` or plain `@register` on an annotated function.

    Curiously, @staticmethod does work, but not as documented (don't decorate the actual implementations):

        from functools import singledispatchmethod
        
        
        class Negator:
            @singledispatchmethod
            @staticmethod
            def neg(arg):
                raise NotImplementedError("Cannot negate a")
        
            @neg.register
            def _(arg: int):
                return -arg
        
            @neg.register
            def _(arg: bool):
                return not arg
        
        
        if __name__ == "__main__":
            print(Negator.neg(0))
            print(Negator.neg(False))

    Leads to

        $ python -m good_staticmethod
        0
        True

    Removing @classmethod from the implementation methods doesn't work, though

        Traceback (most recent call last):
          File "/usr/lib/python3.8/runpy.py", line 193, in _run_module_as_main
            return _run_code(code, main_globals, None,
          File "/usr/lib/python3.8/runpy.py", line 86, in _run_code
            exec(code, run_globals)
          File "/home/viktor/scratch/bad_classmethod_alternative.py", line 20, in <module>
            print(Negator.neg(0))
          File "/usr/lib/python3.8/functools.py", line 911, in _method
            return method.__get__(obj, cls)(*args, **kwargs)
        TypeError: _() missing 1 required positional argument: 'arg'

    @vr2262 vr2262 mannequin added 3.8 only security fixes 3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Feb 18, 2020
    @tirkarthi
    Copy link
    Member

    I guess the method checks for annotation on cls [0] which will be classmethod/staticmethod object in the report and won't have annotations. The annotations should be looked up in the function the classmethod/staticmethod decorator wraps around as in cls.__func__ . Something like below so that the right annotations are picked up. In addition to this the registry should store the type annotation as key to cls or cls.__func__ depending on normal method or classmethod/staticmethod.

    diff --git Lib/functools.py Lib/functools.py
    index 050bec8605..a66711208d 100644
    --- Lib/functools.py
    +++ Lib/functools.py
    @@ -1073,24 +1073,33 @@ def singledispatch(func):
             if func is None:
                 if isinstance(cls, type):
                     return lambda f: register(cls, f)
    -            ann = getattr(cls, '__annotations__', {})
    +            if isinstance(cls, (classmethod, staticmethod)):
    +                ann = getattr(cls.__func__, '__annotations__', {})
    +                func = cls.__func__
    +            else:
    +                ann = getattr(cls, '__annotations__', {})
    +                func = cls

    [0]

    ann = getattr(cls, '__annotations__', {})

    @vr2262
    Copy link
    Mannequin Author

    vr2262 mannequin commented Feb 21, 2020

    I tried to apply this change but it didn't work, failing with this error

        $ ~/.pyenv/versions/3.8.1/bin/python -m bad_classmethod_as_documented
        Traceback (most recent call last):
          File "/home/viktor/.pyenv/versions/3.8.1/lib/python3.8/runpy.py", line 193, in _run_module_as_main
            return _run_code(code, main_globals, None,
          File "/home/viktor/.pyenv/versions/3.8.1/lib/python3.8/runpy.py", line 86, in _run_code
            exec(code, run_globals)
          File "/home/viktor/scratch/bad_classmethod_as_documented.py", line 4, in <module>
            class Negator:
          File "/home/viktor/scratch/bad_classmethod_as_documented.py", line 12, in Negator
            def _(cls, arg: int):
          File "/home/viktor/.pyenv/versions/3.8.1/lib/python3.8/functools.py", line 1006, in register
            return self.dispatcher.register(cls, func=method)
          File "/home/viktor/.pyenv/versions/3.8.1/lib/python3.8/functools.py", line 959, in register
            argname, cls = next(iter(get_type_hints(func).items()))
          File "/home/viktor/.pyenv/versions/3.8.1/lib/python3.8/typing.py", line 1252, in get_type_hints
            raise TypeError('{!r} is not a module, class, method, '
        TypeError: <classmethod object at 0x7f84e1e69c40> is not a module, class, method, or function.

    After digging around a bit, this diff seems to work (not sure if there's a better way to do it) (also this one doesn't seem to care whether @staticmethod is applied to the implementation methods):

    $ diff -u functools-orig.py functools.py
    --- functools-orig.py	2020-02-21 16:14:56.141934001 -0500
    +++ functools.py	2020-02-21 16:17:19.959905849 -0500
    @@ -843,14 +843,18 @@
             if func is None:
                 if isinstance(cls, type):
                     return lambda f: register(cls, f)
    -            ann = getattr(cls, '__annotations__', {})
    +            if isinstance(cls, (classmethod, staticmethod)):
    +                ann = getattr(cls.__func__, '__annotations__', {})
    +                func = cls.__func__
    +            else:
    +                ann = getattr(cls, '__annotations__', {})
    +                func = cls
                 if not ann:
                     raise TypeError(
                         f"Invalid first argument to `register()`: {cls!r}. "
                         f"Use either `@register(some_class)` or plain `@register` "
                         f"on an annotated function."
                     )
    -            func = cls
     
                 # only import typing if annotation parsing is necessary
                 from typing import get_type_hints
    @@ -908,6 +912,8 @@
         def __get__(self, obj, cls=None):
             def _method(*args, **kwargs):
                 method = self.dispatcher.dispatch(args[0].__class__)
    +            if isinstance(self.func, classmethod):
    +                return method.__get__(obj, cls)(cls, *args, **kwargs)
                 return method.__get__(obj, cls)(*args, **kwargs)
     
             _method.__isabstractmethod__ = self.__isabstractmethod__

    @tirkarthi
    Copy link
    Member

    Sorry, I had the part only to detect annotations attached. My part is something similar to yours except it stores the appropriate function in the registry itself instead of passing the arguments at __get__ .

    @vr2262
    Copy link
    Mannequin Author

    vr2262 mannequin commented Jun 10, 2020

    Sorry to bump this after months of inactivity but is there something I need to do here?

    @mgrandi
    Copy link
    Mannequin

    mgrandi mannequin commented Jun 26, 2020

    same issue here, if we can't fix it then maybe we should edit the documentation to not suggest that @classmethod works?

    @glyph
    Copy link
    Mannequin

    glyph mannequin commented Oct 9, 2020

    I also just discovered this. I thought it must have regressed at some point but I see the docs say "new in 3.8" and I'm using 3.8.

    Is there a "no CI for examples in the docs" issue that this could be linked to?

    @EugenePY
    Copy link
    Mannequin

    EugenePY mannequin commented Nov 2, 2020

    Hi, I also encounter to the problem, and I have give my attempt to make the singledispatchmethod support the classmethod, and staticmethod with type annotation. I also adding two tests. Please refer to my fork. I will trying to make a pull request

    EugenePY/cpython@3.8...fix-issue-39679

    @dmkulazhenko
    Copy link
    Mannequin

    dmkulazhenko mannequin commented Jun 16, 2021

    Based on what I've read, workaround:

    from functools import singledispatchmethod
    
    
    def _register(self, cls, method=None):
        if hasattr(cls, "__func__"):
            setattr(cls, "__annotations__", cls.__func__.__annotations__)
        return self.dispatcher.register(cls, func=method)
    
    
    singledispatchmethod.register = _register

    @AlexWaygood
    Copy link
    Member

    AlexWaygood commented Oct 17, 2021

    Happily, this bug appears to have been resolved in Python 3.10 due to the fact that a classmethod wrapping a function F will now have an __annotations__ dict equal to F.

    In Python 3.9:

    >>> x = lambda y: y
    >>> x.__annotations__ = {'y': int}
    >>> c = classmethod(x)
    >>> c.__annotations__
    Traceback (most recent call last):
      File "<pyshell#37>", line 1, in <module>
        c.__annotations__
    AttributeError: 'classmethod' object has no attribute '__annotations__'

    In Python 3.10:

    >>> x = lambda y: y
    >>> x.__annotations__ = {'y': int}
    >>> c = classmethod(x)
    >>> c.__annotations__
    {'y': <class 'int'>}

    This change appears to have resolved the bug in functools.singledispatchmethod. The bug remains in Python 3.9, however.

    @AlexWaygood AlexWaygood removed 3.8 only security fixes labels Oct 17, 2021
    @ambv
    Copy link
    Contributor

    ambv commented Oct 19, 2021

    New changeset ad6d162 by Alex Waygood in branch 'main':
    bpo-39679: Add tests for classmethod/staticmethod singledispatchmethods (GH-29034)
    ad6d162

    @ambv
    Copy link
    Contributor

    ambv commented Oct 19, 2021

    New changeset c15ba30 by Miss Islington (bot) in branch '3.10':
    bpo-39679: Add tests for classmethod/staticmethod singledispatchmethods (GH-29034) (GH-29072)
    c15ba30

    @ambv
    Copy link
    Contributor

    ambv commented Oct 19, 2021

    Thanks for the new tests, Alex. Not closing this just yet because maybe we find a way to fix it in 3.9.8+.

    @ambv
    Copy link
    Contributor

    ambv commented Oct 28, 2021

    New changeset 97388c2 by Alex Waygood in branch '3.9':
    [3.9] bpo-39679: Fix singledispatchmethod classmethod/staticmethod bug (GH-29087)
    97388c2

    @ambv
    Copy link
    Contributor

    ambv commented Oct 28, 2021

    Thanks for the 3.9 fix, Alex! ✨ 🍰 ✨

    @P403n1x87
    Copy link
    Contributor

    Is there a reason why this was not backported to 3.8, when the documentation still says that decorators are supported?

    @AlexWaygood
    Copy link
    Member

    Is there a reason why this was not backported to 3.8, when the documentation still says that decorators are supported?

    This was fixed in October 2021. Python 3.8 was old enough to enter "security-only" mode in May 2021. Python versions in security-only mode only have docs updates or bugfixes applied to them if the docs update or bugfix relates to a security issue. This bug didn't relate to a security issue.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only 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

    4 participants