Skip to content

Bound instance methods fail to satisfy runtime_checkable Protocol #127059

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

Closed
GDYendell opened this issue Nov 20, 2024 · 3 comments
Closed

Bound instance methods fail to satisfy runtime_checkable Protocol #127059

GDYendell opened this issue Nov 20, 2024 · 3 comments
Labels
3.12 only security fixes 3.13 bugs and security fixes 3.14 bugs and security fixes topic-typing type-bug An unexpected behavior, bug, or error

Comments

@GDYendell
Copy link

GDYendell commented Nov 20, 2024

Bug report

Bug description:

#103034 added some protections around isinstance checks of runtime_checkable Protocols using inspect.getattr_static. This uses inspect._check_instance which uses object.__getattribute__(obj, "__dict__"), which fails for bound instance methods (although obj.__dict__ does work), so they cannot fulfill runtime_checkable Protocols anymore.

from typing import Protocol, runtime_checkable


@runtime_checkable
class Labelled(Protocol):
    label: str


class A:
    def f(self):
        pass


A.f.label = "f"

a = A()
print(a.f.__dict__)  # {'label': 'f'}
print(object.__getattribute__(a.f, "__dict__"))  # AttributeError

assert isinstance(a.f, Labelled)  # AssertionError

Is this an intended/known change? Could it be noted in the changelog here as an example of case that will no longer work?


Created from #103034 (comment)
cc @AlexWaygood

CPython versions tested on:

3.12

Operating systems tested on:

Linux

@GDYendell GDYendell added the type-bug An unexpected behavior, bug, or error label Nov 20, 2024
@AlexWaygood AlexWaygood added topic-typing 3.12 only security fixes 3.13 bugs and security fixes 3.14 bugs and security fixes labels Nov 20, 2024
@GDYendell
Copy link
Author

Thanks @AlexWaygood. I am happy to help with this if I am able, but I imagine the first step is deciding what (if anything) to do about it!

@AlexWaygood
Copy link
Member

AlexWaygood commented Nov 20, 2024

My first question here was "So why doesn't the object.__getattribute__(x, "__dict__") call work when x is a bound instance method? And it seems like the answer is that types.MethodType overrides object.__getattribute__:

>>> class Foo: pass
...
>>> Foo.__getattribute__ is object.__getattribute__
True
>>> import types
>>> types.MethodType.__getattribute__ is object.__getattribute__
False

My next question was, "Why does MethodType override object.__getattribute__?" And it looks like this is so that bound methods forward attribute access to the underlying function object:

>>> class Foo:
...     def f(self): pass
...     
>>> Foo.f.bar = 42
>>> 
>>> Foo.f.__dict__ is Foo().f.__dict__
True
>>> Foo().f.bar
42

inspect.getattr_static is therefore working as intended here, since the whole point of inspect.getattr_static is that -- unlike regular attribute access -- it will not resolve dynamic attribute lookup implemented via special __getattribute__ methods and similar. Resolving dynamic attribute lookup can execute arbitrary code, and that's exactly what inspect.getattr_static goes out of its way to avoid.

I think the choice to switch to getattr_static was still the correct one overall, so it doesn't seem like the behaviour of runtime-checkable protocols can be changed so that this once again works for you like it used to. Therefore the third question is, "Should we update the docs to explicitly note the problem here with bound instance methods?"

I'm not sure we should change the docs here. We already have a note in "What's new" outlining that the change we made might cause problems like this. I'm not sure this specific case should be called out there, as it seems like something of an edge case that not many users are likely to run into. Python 3.12 has now been released for over a year, and this is the first time anybody has raised this issue. Even for "What's new", it's important to keep the document focussed and streamlined.

So, to summarize: I'm really sorry that this change broke your code! But, I'm not sure there's anything to be done here :(

@GDYendell
Copy link
Author

This all makes sense. I was actually part way through refactoring my code for compliance with strict pyright (which does not allow setting arbitrary attributes on functions) when I discovered it doesn't run on 3.12, so this is something that will go away soon in my case. I mainly thought it was interesting and wondered if it was a bug. Hopefully this issue will now appear in searches and your excellent summary above will provide the documentation of this behaviour.

Thanks for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 only security fixes 3.13 bugs and security fixes 3.14 bugs and security fixes topic-typing type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

2 participants