Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 33 additions & 27 deletions Lib/functools.py
Original file line number Diff line number Diff line change
Expand Up @@ -1026,9 +1026,6 @@ def __init__(self, func):
self.dispatcher = singledispatch(func)
self.func = func

import weakref # see comment in singledispatch function
self._method_cache = weakref.WeakKeyDictionary()

def register(self, cls, method=None):
"""generic_method.register(cls, func) -> func

Expand All @@ -1037,36 +1034,45 @@ def register(self, cls, method=None):
return self.dispatcher.register(cls, func=method)

def __get__(self, obj, cls=None):
if self._method_cache is not None:
try:
_method = self._method_cache[obj]
except TypeError:
self._method_cache = None
except KeyError:
pass
else:
return _method
return _singledispatchmethod_get(self, obj, cls)

dispatch = self.dispatcher.dispatch
funcname = getattr(self.func, '__name__', 'singledispatchmethod method')
def _method(*args, **kwargs):
if not args:
raise TypeError(f'{funcname} requires at least '
'1 positional argument')
return dispatch(args[0].__class__).__get__(obj, cls)(*args, **kwargs)
@property
def __isabstractmethod__(self):
return getattr(self.func, '__isabstractmethod__', False)

_method.__isabstractmethod__ = self.__isabstractmethod__
_method.register = self.register
update_wrapper(_method, self.func)

if self._method_cache is not None:
self._method_cache[obj] = _method
class _singledispatchmethod_get:
def __init__(self, unbound, obj, cls):
self._unbound = unbound
self._dispatch = unbound.dispatcher.dispatch
self._obj = obj
self._cls = cls

return _method
def __call__(self, /, *args, **kwargs):
if not args:
funcname = getattr(self._unbound.func, '__name__',
'singledispatchmethod method')
raise TypeError(f'{funcname} requires at least '
'1 positional argument')
return self._dispatch(args[0].__class__).__get__(self._obj, self._cls)(*args, **kwargs)

def __getattr__(self, name):
if name not in {'__name__', '__qualname__', '__isabstractmethod__',
'__annotations__', '__type_params__'}:
Comment on lines +1073 to +1074
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this set is functools.WRAPPER_ASSIGNMENTS minus __doc__ and __module__ which have special handling. Fine to leave it this way

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plus __isabstractmethod__.

raise AttributeError
return getattr(self._unbound.func, name)

@property
def __isabstractmethod__(self):
return getattr(self.func, '__isabstractmethod__', False)
def __module__(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is giving some problems in my interactive console. A minimal reproducer:

from functools import singledispatchmethod, singledispatch
from IPython.lib.pretty import pretty

class A:
    def __init__(self, value):
        self.value = value
        
    @singledispatchmethod
    def dp(self, x):
        return id(self)
    
a=obj=A(4)

pretty(a.dp) # fails

_singledispatchmethod_get.__module__ # this is now a property, not a string

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there is a problem with type attributes which we want to define for instances (__name__, __qualname__, __doc__). If we define a property, it conflicts with the type attribute.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be resolved by defining __getattribute__ instead of __getattr__. But this adds such large overhead, that it outweigh the optimization gain.

So I added setting just two instance attributes __module__ and __doc__ in the constructor. This adds some overhead, but not so large as in the original code. I hope that more satisfying solution will be found in future, but this is a complex issue.

return self._unbound.func.__module__

@property
def __doc__(self):
return self._unbound.func.__doc__

@property
def register(self):
return self._unbound.register


################################################################################
Expand Down
61 changes: 61 additions & 0 deletions Lib/test/test_functools.py
Original file line number Diff line number Diff line change
Expand Up @@ -2900,6 +2900,7 @@ def static_func(arg: int) -> str:
"""My function docstring"""
return str(arg)

prefix = A.__qualname__ + '.'
for meth in (
A.func,
A().func,
Expand All @@ -2909,6 +2910,8 @@ def static_func(arg: int) -> str:
A().static_func
):
with self.subTest(meth=meth):
self.assertEqual(meth.__module__, __name__)
self.assertEqual(meth.__qualname__, prefix + meth.__name__)
self.assertEqual(meth.__doc__,
('My function docstring'
if support.HAVE_DOCSTRINGS
Expand Down Expand Up @@ -3241,6 +3244,64 @@ def f(arg):
def _(arg: undefined):
return "forward reference"

def test_method_equal_instances(self):
# gh-127750: Reference to self was cached
class A:
def __eq__(self, other):
return True
def __hash__(self):
return 1
@functools.singledispatchmethod
def t(self, arg):
return self

a = A()
b = A()
self.assertIs(a.t(1), a)
self.assertIs(b.t(2), b)

def test_method_bad_hash(self):
class A:
def __eq__(self, other):
raise AssertionError
def __hash__(self):
raise AssertionError
@functools.singledispatchmethod
def t(self, arg):
pass

# Should not raise
A().t(1)
hash(A().t)
A().t == A().t

def test_method_no_reference_loops(self):
# gh-127750: Created a strong reference to self
class A:
@functools.singledispatchmethod
def t(self, arg):
return weakref.ref(self)

a = A()
r = a.t(1)
self.assertIsNotNone(r())
del a # delete a after a.t
if not support.check_impl_detail(cpython=True):
support.gc_collect()
self.assertIsNone(r())

a = A()
t = a.t
del a # delete a before a.t
support.gc_collect()
r = t(1)
self.assertIsNotNone(r())
del t
if not support.check_impl_detail(cpython=True):
support.gc_collect()
self.assertIsNone(r())


class CachedCostItem:
_cost = 1

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Remove broken :func:`functools.singledispatchmethod` caching introduced in
:gh:`85160`. Achieve the same performance using different optimization.
Loading