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

[stubgen] align is_classmethod with is_staticmethod #18657

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ericmarkmartin
Copy link
Contributor

@ericmarkmartin ericmarkmartin commented Feb 11, 2025

Make classmethod detection more in line with staticmethod detection. While the current check of inspect.isbuiltin works most of the time, there are occasions where it doesn't work. For instance, callables created via PyCMethod_New and friends return True under inspect.isbuiltin.

Also previously, I don't believe the check that the the object had type classmethod or classmethod_descriptor was ever triggering, as the object was being looked up via getattr and therefore with the descriptor protocol. Now, we use a raw lookup so we can actually get a hold of the descriptor.

@ericmarkmartin ericmarkmartin force-pushed the improve-inspection-is_classmethod branch from 7de3489 to ea4bbf0 Compare February 11, 2025 05:39
@JukkaL JukkaL changed the title align is_classmethod with is_staticmethod [stubgen] align is_classmethod with is_staticmethod Feb 12, 2025
@ericmarkmartin ericmarkmartin force-pushed the improve-inspection-is_classmethod branch 5 times, most recently from 7094d74 to 472917d Compare February 22, 2025 05:25
ericmarkmartin and others added 2 commits February 22, 2025 00:25
Make classmethod detection more in line with staticmethod detection.
This correctly handles a few additional cases: for example, callables
created via PyCMethod_New and friends will no longer be classified as
classmethods.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants