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

3.13.0b3: undocumented change in inspect.isroutine() for functools.partial #122087

Closed
mgorny opened this issue Jul 21, 2024 · 7 comments
Closed
Labels
docs Documentation in the Doc dir release-blocker

Comments

@mgorny
Copy link
Contributor

mgorny commented Jul 21, 2024

Documentation

While investigating encode/starlette#2648, I've noticed that the behavior of inspect.isroutine() changed with regards to functools.partial().

Please consider the following sample:

import inspect
from functools import partial

def foo(x):
    pass

p = partial(foo, 1)
print(inspect.isroutine(p))

Prior to 3.13.0b3, it would evaluate to False. In newer versions, it evaluates to True.

A bisect points out that it changed as a result of 49e5740 (i.e. #121086). Neither the commit message, nor the news entry, hints anything about inspect.isroutine() behavior.

Could you please clarify whether the change was intentional? And if it was, could we have it explicitly noted in the documentation?

Linked PRs

@mgorny mgorny added the docs Documentation in the Doc dir label Jul 21, 2024
@mgorny
Copy link
Contributor Author

mgorny commented Jul 21, 2024

CC @serhiy-storchaka

@hugovk
Copy link
Member

hugovk commented Jul 21, 2024

I added the release-blocker label to make sure this is addressed one way or another before RC1 on the 30th.

@ncoghlan
Copy link
Contributor

ncoghlan commented Jul 22, 2024

Reviewing the issue, I suspect functools.partial may need further special-casing in inspect for 3.13 (the addition of the descriptor method doesn't currently change its behaviour, but some of the inspect functions are already treating it as if the 3.14 change was already in effect)

In the backport PR, https://github.com/python/cpython/pull/121092/files#diff-b89cd06e4637abacb73f2500301ff979a67ad7ecbfa7cf151c7243715898d7eaL2558 adjusted _signature_from_callable to avoid having the addition of __get__ change the reported signature, which was enough to keep CI happy, but it appears that doesn't cover the whole of the inspect module when it comes to partial object handling.

@encukou
Copy link
Member

encukou commented Jul 23, 2024

@serhiy-storchaka, will you be able to look at this before rc1?

@serhiy-storchaka
Copy link
Member

Unfortunately, the only way to add a warning about a partial object being made a method descriptor in the future is to make it a method descriptor. We can make inspect.ismethoddescriptor() lying about partial objects, but this will not help the user code which check the __get__ method directly. I do not know how to break this loop.

@mgorny
Copy link
Contributor Author

mgorny commented Jul 24, 2024

What's the expected result in 3.14? Is partial() there expected to be recognized as a routine, and not have a __name__ attribute? Ideally, is there some documentation that explains whether __name__ is expected on all routines or not?

@serhiy-storchaka
Copy link
Member

Yes, not all method descriptors (and therefore not all routines) have the __name__ attribute. This always was so, you always should have a fallback.

#122218 add a special case for partial in ismethoddescriptor() and tests for 3.13. #122219 adds corresponding tests for main branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir release-blocker
Projects
Development

No branches or pull requests

5 participants