-
Notifications
You must be signed in to change notification settings - Fork 52
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
Better docstring linker context inferface #599
Better docstring linker context inferface #599
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #599 +/- ##
==========================================
- Coverage 92.47% 92.40% -0.07%
==========================================
Files 47 47
Lines 8170 8060 -110
Branches 1948 1926 -22
==========================================
- Hits 7555 7448 -107
+ Misses 358 357 -1
+ Partials 257 255 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
This is ready to be merged. |
I’ll merge this PR, need this to fix other issues. |
@adiroiban can I have a approval for this PR since it trashes a lot of the linker code? |
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.
This looks… fine, I think… but some of the docstrings and comments need some help.
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.
Hi @glyph, thanks a lot for your review and sorry about the poor quality of the docstrings.
I’ll merge this PR. |
We 're good for this PR @glyph |
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.
LGTM
Related to #295, as well as #661 and #662.
This change allow us to output correct link no matter the docstring linker we use to render on what page of the documentation. Historically, the page object was always the one associated with the docstring linker's object (passed on creation), but now we make the differences in between both concepts.
I’ve also totally removed the caching system of xref links. It’s rather complex and it did not improve performance.