-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
autodoc: fix detection of class methods implemented by extension modules #13200
base: master
Are you sure you want to change the base?
Conversation
Works. |
Btw, I'm not sure if this is something CPython wants to improve by adding some |
Can't you use CPython builtin classes (e.g. int) to test this? BTW, there is no "official" way to describe signatures for extensions. The gmp module (as many CPython builtins) uses funny notation in |
I can use built-in classes but I wanted some complex inheritance just in case. I guess I can write some tests with what I have already in hand though. |
IIRC, multiple inheritance does not work for statically defined types. |
Oh I see. Though, I thought about inheriting (in pure Python) from my custom built-in class just to check that we correctly pick up something written in C as well. What I didn't want to do is use a built-in whose signature rendering may change across versions for various reasons. It makes tests harder to maintain (something that usually happens for |
# Conflicts: # CHANGES.rst # sphinx/ext/autodoc/__init__.py
I agree with Sergey that using e.g. A |
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
sphinx/util/inspect.py
Outdated
return is_classmethod_descriptor(obj, meth, name) or ( | ||
isbuiltin(obj) | ||
and getattr(obj, '__self__', None) is not None | ||
and isclass(obj.__self__) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously we had is_builtin_classmethod(meth)
, so cls=None
and name=None
, so the inlined code would actually be:
return is_classmethod_descriptor(obj, meth, name) or ( | |
isbuiltin(obj) | |
and getattr(obj, '__self__', None) is not None | |
and isclass(obj.__self__) | |
) | |
return is_classmethod_descriptor(meth, None, None) or ( | |
isbuiltin(meth) | |
and getattr(meth, '__self__', None) is not None | |
and isclass(meth.__self__) | |
) |
Sorry for the error. However it seems that there is no test coverage of this as tests passed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we exercise this code path when obj
is a junk object but we know the base class and the name (e.g., obj = None
because we tried to retrieve on the __dict__
of a child class).
I can try to write a test by the end of the week (I'm leaving tomorrow for a few days; if I manage to write something tomorrow, I'll add a commit, otherwise I'll only be able to write something on Friday or later)
Closes #13188.
cc @skirpichev
@AA-Turner do we have tests for this? I tested it locally but I'm not sure we're building a C extension module in Sphinx to test this.