Skip to content

feat: Parse sphinx parameter types as Expr expressions #392

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

Merged
merged 5 commits into from
Jul 9, 2025

Conversation

echoix
Copy link
Contributor

@echoix echoix commented Jul 7, 2025

This PR enhances the sphinx docstring style by parsing the type annotations as an expression, when given either with :param a_type foo: or a :param foo:+:type foo: a_type pair. It uses the docstrings.utils.parse_docstring_annotation() function, that is also used in the google docstring style parser file.

Used on a real project, this means that the parameters types in the table can become cross references or links, event if they aren't coming from the annotations of the function signature. Before this PR, only the typing annotations in the signature render to links.

For example, on a simple mkdocs configuration for the astroid project (the one I want to use mkdocstrings with is too complex to know if problems are caused by us or mkdocstrings), this gives:
Before:
image
image

After:
image
image
image

Some tests were added with the expected expression objects, but without the parent= defined for ExprName object types, as type checking complains, see #391. I noticed that the assertions using as_dict() still pass even if the parents are different, so I'm not sure if it is a bug or they are ignored.

Please make sure the asserts added at the end are sufficient, and the ones there are needed. I played around a bit when writing them. Please also make sure I correctly used the expressions as intended.

I have another feature/fix queued up related to this, when a :param a_type foo: has a type that contains spaces (that is not supported by the (upstream) sphinx project either, see), that ended up skipping the parameter in the generated docs, while the info was there. The test added for this helps to cover more cases related to this PR. Because of #391, the test added fails. Since the fix is kinda of another feature, I excluded it from this PR, my first to this repo.

@echoix
Copy link
Contributor Author

echoix commented Jul 7, 2025

Oh, and on 3.14 there were some tests that acted differently because of the way typing.Union is handled. See https://docs.python.org/3.14/whatsnew/3.14.html#typing

Copy link
Member

@pawamoy pawamoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, thanks a lot! Just a few nitpicks.

@pawamoy
Copy link
Member

pawamoy commented Jul 8, 2025

Oh, and on 3.14 there were some tests that acted differently because of the way typing.Union is handled. See https://docs.python.org/3.14/whatsnew/3.14.html#typing

Yep I need to fix that 🙂

Copy link
Member

@pawamoy pawamoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few more, let me batch-commit them and we're good to merge 🙂

@pawamoy pawamoy merged commit 70dda21 into mkdocstrings:main Jul 9, 2025
2 checks passed
@pawamoy
Copy link
Member

pawamoy commented Jul 9, 2025

Thank you! 🚀

@echoix
Copy link
Contributor Author

echoix commented Jul 9, 2025

Just a few more, let me batch-commit them and we're good to merge 🙂

There’s other tests (not in the diff) that could have been changed, i limited myself to the scope of the PR

@pawamoy
Copy link
Member

pawamoy commented Jul 9, 2025

Yep that's good 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants