cpython icon indicating copy to clipboard operation
cpython copied to clipboard

Changed behavior of singledispatch in Python 3.12

Open AdrianSosic opened this issue 1 year ago • 3 comments

Bug report

Bug description:

It seems the behavior of functools.singledispatch changed in Python 3.12 in situations where a typing.Protocol is involved.

When executing the following code on 3.12, MyClass no longer gets dispatched as via the default but as MyProtocol. The problem seems to be that issubclass now behaves differently when called on protocols. The issue was noticed here where it caused downstream problems.

from typing import Protocol
from functools import singledispatch


class MyProtocol(Protocol):
    pass

class MyClass:
    pass

s = singledispatch(lambda x: "default")
s.register(MyProtocol, lambda x: "protocol")

print(s(MyClass))  # yields "default" on 3.11 but "protocol" on 3.12

CPython versions tested on:

3.11, 3.12

Operating systems tested on:

macOS

AdrianSosic avatar Mar 20 '24 19:03 AdrianSosic

cc @AlexWaygood

sobolevn avatar Mar 20 '24 21:03 sobolevn

Okay, I strongly suspect this is because of this delightful part of typing.py:

https://github.com/python/cpython/blob/8ad88984200b2ccddc0a08229dd2f4c14d1a71fc/Lib/typing.py#L1784-L1790

I'll investigate more tomorrow.

AlexWaygood avatar Mar 20 '24 21:03 AlexWaygood

Thanks for the clear bug report, @AdrianSosic!

AlexWaygood avatar Mar 20 '24 21:03 AlexWaygood

Hi @AlexWaygood, just wanted to ask if you could already find some time to figure out what exactly causes the problem? 🙃

AdrianSosic avatar Mar 27 '24 17:03 AdrianSosic

Sorry, I haven't had a chance to look at this properly yet. But thanks for the ping. Please feel free to ping me again if I haven't responded in ~2 weeks :-)

AlexWaygood avatar Mar 27 '24 18:03 AlexWaygood

@AlexWaygood, as requested: ping ping 😅 don't want to annoy you so feel free to tell me you have no time for it 😊

AdrianSosic avatar Apr 10 '24 17:04 AdrianSosic

Thanks, you're not annoying me!

AlexWaygood avatar Apr 10 '24 17:04 AlexWaygood

Just lurking around a little bit … (disappears into nowhere)

AdrianSosic avatar Apr 22 '24 18:04 AdrianSosic

If I haven't got to it by then, I'll definitely look at this at the PyCon Sprints (week commencing 20th May)

AlexWaygood avatar Apr 23 '24 12:04 AlexWaygood

At least two commits changed the behaviour of this snippet between Python 3.11 and Python 3.12:

  • The first commit that changed behaviour was b27fe67f3c643e174c3619b669228ef34b6d87ee. Prior to this commit, the snippet printed "default"; after this commit, the snippet raised TypeError.
  • The second commit that changed behaviour was 076f3cda140a45b08c2c9518bc19624aae50d3a3. Prior to this commit, the snippet raised TypeError; after this commit, the snippet printed "protocol".

In between those two commits, the traceback was as follows:

Traceback (most recent call last):
  File "/Users/alexw/dev/cpython/Lib/functools.py", line 832, in dispatch
    impl = dispatch_cache[cls]
           ~~~~~~~~~~~~~~^^^^^
  File "/Users/alexw/dev/cpython/Lib/weakref.py", line 415, in __getitem__
    return self.data[ref(key)]
           ~~~~~~~~~^^^^^^^^^^
KeyError: <weakref at 0x104d2cc20; to 'type' at 0x104aabfd0 (type)>

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/alexw/dev/cpython/Lib/functools.py", line 835, in dispatch
    impl = registry[cls]
           ~~~~~~~~^^^^^
KeyError: <class 'type'>

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/alexw/dev/cpython/foo.py", line 14, in <module>
    print(s(MyClass))
          ^^^^^^^^^^
  File "/Users/alexw/dev/cpython/Lib/functools.py", line 909, in wrapper
    return dispatch(args[0].__class__)(*args, **kw)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/alexw/dev/cpython/Lib/functools.py", line 837, in dispatch
    impl = _find_impl(cls, registry)
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/alexw/dev/cpython/Lib/functools.py", line 784, in _find_impl
    mro = _compose_mro(cls, registry.keys())
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/alexw/dev/cpython/Lib/functools.py", line 745, in _compose_mro
    types = [n for n in types if is_related(n)]
                                 ^^^^^^^^^^^^^
  File "/Users/alexw/dev/cpython/Lib/functools.py", line 744, in is_related
    and issubclass(cls, typ))
        ^^^^^^^^^^^^^^^^^^^^
  File "/Users/alexw/dev/cpython/Lib/typing.py", line 1799, in __subclasscheck__
    return super().__subclasscheck__(other)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen abc>", line 123, in __subclasscheck__
  File "/Users/alexw/dev/cpython/Lib/typing.py", line 1882, in _proto_hook
    raise TypeError("Instance and class checks can only be used with"
TypeError: Instance and class checks can only be used with @runtime_checkable protocols

AlexWaygood avatar May 23 '24 21:05 AlexWaygood

My feeling is that the new behaviour is probably correct. Calling isinstance() and issubclass() against a protocol that isn't decorated with @runtime_checkable isn't usually allowed, but we make an explicit exception for the functools module. But if you create an empty protocol -- as you're doing here -- and decorate it with @runtime_checkable, then you'll see that all objects are considered instances of that protocol. This has been the case since runtime-checkable protocols were originally introduced in Python 3.8:

Python 3.8.18 (default, Feb 15 2024, 19:36:58) 
[Clang 15.0.0 (clang-1500.1.0.2.5)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from typing import Protocol, runtime_checkable
>>> @runtime_checkable
... class EmptyProtocol(Protocol): pass
... 
>>> isinstance([], EmptyProtocol)
True
>>> isinstance(object(), EmptyProtocol)
True
>>> issubclass(dict, EmptyProtocol)
True
>>> issubclass(object, EmptyProtocol)
True

As such, it makes sense that s.register(MyProtocol, lambda x: "protocol") would lead to lambda x: "protocol" being selected as the relevant function variant for any arbitrary object being passed in. Non-empty protocols also appear to be behaving correctly:

>>> from functools import singledispatch
>>> s = singledispatch(lambda x: "default")
>>> from typing import SupportsIndex
>>> _ = s.register(SupportsIndex, lambda x: "supports index")
>>> s('foo')
'default'
>>> s({})
'default'
>>> s(432)
'supports index'

So, I think the only thing to do here is to add a regression test, since it appears that there was a bug on Python <=3.11 that was accidentally fixed. @carljm / @JelleZijlstra, do you agree?

AlexWaygood avatar May 23 '24 22:05 AlexWaygood

I agree that everything should match an empty protocol and therefore the new behavior is correct.

However, it seems wrong that singledispatch allows dispatching on a non-runtime checkable protocol. I tried locally and test_functools and test_typing both pass if I remove functools from _allow_reckless_class_checks. I wonder if we can remove it.

JelleZijlstra avatar May 23 '24 22:05 JelleZijlstra

However, it seems wrong that singledispatch allows dispatching on a non-runtime checkable protocol. I tried locally and test_functools and test_typing both pass if I remove functools from _allow_reckless_class_checks. I wonder if we can remove it.

I agree that this does seem wrong. However, changing it will break users like @AdrianSosic who have been relying on the existing behaviour where it was allowed. I don't think we can change this without a deprecation period.

AlexWaygood avatar May 23 '24 22:05 AlexWaygood