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._parse_annotation should improve formatting of Union, Optional, Literal, constants #9643

Closed
jbms opened this issue Sep 16, 2021 · 6 comments · Fixed by #11109
Labels
domains:py type:enhancement enhance or introduce a new feature
Milestone

Comments

@jbms
Copy link
Contributor

jbms commented Sep 16, 2021

This is a sub-issue of #9523 split off here.

When displayed normally, Union, Optional, and Literal add a lot of noise to the type signature and obscure the important information. Instead, it is much cleaner to display them using the PEP 604 (https://www.python.org/dev/peps/pep-0604/) syntax:

Union[X, Y, Z] -> X | Y | Z
Optional[X] -> X | None

Additionally, for Literal it is cleaner to strip the text "Literal" and just display the literal value with normal Python syntax highlighting:

Literal["X"] -> "X"

This is implemented in the tensorstore documentation via an ast transformation:

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

This should be supported in Sphinx via a config option. The other improvement, of using syntax highlighting for constants, should also be integrated.

@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

I think the former proposal is cool. It's good for me. +1 for adding the option. At this moment, some libraries and applications must support old intepreters that does not supporting type union operator. So it's useful until the deprecation of them.

On the other hand, I'm not sure another proposal is really good. The converted code is not valid in Python. I agree "Literal" text is not meaningless in the document. But I'm afraid the invalid code confuses readers. Could you show any rendered example? I'd like to see a real example.

@jbms
Copy link
Contributor Author

jbms commented Sep 17, 2021

Here is an example of the Literal transformation:

https://google.github.io/tensorstore/python/api/tensorstore.TensorStore.read.html#p-order

The order parameter is annotated with the type Optional[Literal['C','F']].

While it is invalid Python syntax, it is unambiguous, except in the case of Literal[T], where T is itself a type. But in that case you could instead use the more customary form Type[T] rather than Literal[T].

@tk0miya
Copy link
Member

tk0miya commented Sep 18, 2021

Thank you for the pointer. Indeed, it's not so bad. Personally, I prefer using Literal. But it would be a candidate of the format of API references. +0 to adding an option for it.

@shimizukawa Do you have any comment for this?

@shimizukawa
Copy link
Member

I think the former proposal is cool. It's good for me. +1 for adding the option.

+1 from me too.

order: 'C' | 'F' = 'C'

+0.
If the official Python documentation generated documentation using this feature, they would use Literal['C','F']. However, it is understandable that someone would want to choose ease of communication over accuracy. It would be good if document writer could choose whether to output Literal['C','F'] or 'C' | 'F'.

@AA-Turner
Copy link
Member

@jbms #11072 does this for Optional and Union; though avoids changing Literal as no "short" display mechanism has been adopted for upstream Python use. I'd be interested in your view here of what (if anything) we should do re literals -- add an option for the short display format you propose, do nothing, etc.

Thanks!

A

@jbms
Copy link
Contributor Author

jbms commented Jan 7, 2023

Thanks for adding the optional/union change. For literal I think it would make sense to add a config option that defaults to false, as we've done in sphinx-immaterial:

https://jbms.github.io/sphinx-immaterial/apidoc/python/index.html#confval-python_transform_type_annotations_concise_literal

Note that the concise literal formatting is unambiguous when used in documentation, because we never represent type names as string literals. However, in Python source code, a real string literal would be ambiguous with specifying a type name as a string literal.

More generally, it would also be nice if there were a mechanism for extensions to manipulate the ast of type annotations before they are formatted, either a sphinx event or something else, since it is somewhat difficult to accomplish that with monkey patching.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
domains:py type:enhancement enhance or introduce a new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants