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

sphinx.domains.python.PyXrefMixin.make_xrefs should be consistent with _parse_annotation #9641

Open
jbms opened this issue Sep 16, 2021 · 3 comments
Labels
domains:py type:enhancement enhance or introduce a new feature

Comments

@jbms
Copy link
Contributor

jbms commented Sep 16, 2021

Sub-issue split off from #9523.

The tensorstore documentation theme changes sphinx.domains.python.PyXrefMixin.make_xrefs to just use _parse_annotation, so that the formatting of types is the same in both signatures and in the field lists (e.g. "parameters", "return type", "raises" listings):

https://github.com/google/tensorstore/blob/1a59fcb310bc1feb13569f03f7134b4c3a5fa5f4/docs/tensorstore_sphinx_ext/autodoc.py#L76

Currently make_xrefs has a separate implementation, perhaps to accommodate more free-form syntax used in older parameter type specifications before the introduction of type annotations, like "int or float":

def make_xrefs(self, rolename: str, domain: str, target: str,

I think the older-style "int or float" style should be discourage now that type annotations are so widely used. However, if it is desired to still support them, make_xrefs should still dispatch to _parse_annotations so that real type annotations are displayed consistently.

@jbms jbms added the type:enhancement enhance or introduce a new feature label Sep 16, 2021
@tk0miya
Copy link
Member

tk0miya commented Sep 17, 2021

Reasonable. It would be better to support new-style annotations. At the same time, we also support old-styled old-styled type explanations written by narrative. I believe they should be also supported. I believe they're legacy. So it's better to add an option to switch the behavior.

@jbms
Copy link
Contributor Author

jbms commented Sep 17, 2021

Maybe the old-style syntax and real type annotation syntax can both be supported at the same time, without even needing an option?

Do you know what the old-style syntax can be? Can we assume it is just type annotations separated by or ?

@tk0miya
Copy link
Member

tk0miya commented Sep 18, 2021

Do you know what the old-style syntax can be? Can we assume it is just type annotations separated by or ?

It's described here: https://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html#info-field-lists.
But it does not mean all documents follow this rule because the :type: field takes a free text, not a type.

@AA-Turner AA-Turner added this to the some future version milestone Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domains:py type:enhancement enhance or introduce a new feature
Projects
None yet
Development

No branches or pull requests

3 participants