gh-131116: Fix inspect._finddoc to work with cached_property objects. #131165
Conversation
lincolnj1
changed the title
|
In Lib/test/test_inspect/inspect_fodder.py, the last class has a bunch of comments where the comment includes the specific line number, i.e. I'm having trouble getting the test suite working locally to get a sense of if that matters at all, but I'm guessing that some |
I believe I fully reverted that in my later commits. There should no longer be any changes to inspect_fodder.py. Which commit have you pulled? Those comments are there to test the comment extractor part of inspect.py. After a bit of testing I felt adding new stuff to the fodder file required changing more hard coded numbers than I thought was worthwhile, so I made a new fodder file. I'm also having trouble getting the test suite working after my last pull from main. Are you getting the same "cannot import name 'Union' from '_typing' (unknown location)" error? |
|
ope, my bad, was looking at a commit and spent so long trying to get the devcontainer to work you had made new updates since then that I didn't look at. |
There was a problem hiding this comment.
can you please check the failing tests here please, Tests / Hypothesis tests on Ubuntu (pull_request) ? if they are relevant failures?
Syncing with main this afternoon seems to have fixed it. I'm not sure why it was failing in the first place, since it seems to test code that I didn't modify at all. The tests ran fine on my Ubuntu terminal before and after I submitted so that was a bit baffling to me. |
|
The test is flaky I think (we've seen similar failures elsewhere) |
|
@picnixz Do you have any thoughts on this pr? It's been a couple weeks and I'm hoping for core dev to look at it. |
|
I don't have time until the end of the week so I'm requesting myself a review |
| from functools import cached_property | ||
|
|
||
| class Parent: | ||
| @cached_property | ||
| def foo(self): | ||
| "this is the docstring for foo" | ||
| pass | ||
|
|
||
| class Child(Parent): | ||
| @cached_property | ||
| def foo(self): | ||
| pass |
There was a problem hiding this comment.
| from functools import cached_property | |
| class Parent: | |
| @cached_property | |
| def foo(self): | |
| "this is the docstring for foo" | |
| pass | |
| class Child(Parent): | |
| @cached_property | |
| def foo(self): | |
| pass | |
| from functools import cached_property | |
| class Parent: | |
| @cached_property | |
| def foo(self): | |
| """this is the docstring for foo""" | |
| class Child(Parent): | |
| @cached_property | |
| def foo(self): | |
| pass |
Can we also have a test where the parent cached property has no docstring but the child does? and maybe if the child redefines foo but as a non-cached property?
| self.assertEqual(inspect.getdoc(mod3.Parent.foo), | ||
| inspect.getdoc(mod3.Child.foo)) |
There was a problem hiding this comment.
Please, check against exact strings
| @@ -0,0 +1 @@ | |||
| inspect.getdoc(), if run on a cached_property that does not define a docstring but inherits from a class that did define a docstring, will now return the inherited docstring rather than None. | |||
There was a problem hiding this comment.
| inspect.getdoc(), if run on a cached_property that does not define a docstring but inherits from a class that did define a docstring, will now return the inherited docstring rather than None. | |
| :func:`inspect.getdoc` now correctly returns a inherited docstring on :class:`~functools.cached_property` objects if none is given in a subclass. |
This also needs a versionchanged:: next for inspect.getdoc
Calling inspect.getdoc on a cached_property that inherits a docstring and does not define one would return None rather than the inherited docstring. This is because inspect._finddoc would treat the cached_property obj as a methoddescriptor and encounter an AttributeError trying to access obj.__name__. This PR adds a special case for cached_properties in _finddoc so that it can find the inherited docstring.
inspect.getdoc()failing to return docstring for cached_property for subclass that overrides code but not docstring #131116