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

Types with nested "|" (Union) are not formatted well #424

Closed
hhoppe opened this issue Aug 3, 2022 · 6 comments
Closed

Types with nested "|" (Union) are not formatted well #424

hhoppe opened this issue Aug 3, 2022 · 6 comments
Labels

Comments

@hhoppe
Copy link

hhoppe commented Aug 3, 2022

For this program:

from __future__ import annotations
from typing import Iterable

def function1(value: int | str = 1) -> None:
  pass

def function2(value: int | str | Iterable[int | str] = 1) -> None:
  pass

pdoc produces the documentation:

def function1(value: int | str = 1) -> None:

def function2(value: Union[int, str, Iterable[int | str]] = 1) -> None:

If feasible, it might be best to always use the more modern ... | ... syntax and never the Union[...] syntax.

@hhoppe hhoppe added the bug label Aug 3, 2022
@mhils
Copy link
Member

mhils commented Aug 3, 2022

This seems to be an upstream bug in Python itself:

>>> from typing import Iterable
>>> int | str | Iterable[int | str]
typing.Union[int, str, typing.Iterable[int | str]]

as well as

>>> import inspect
>>> inspect.formatannotation(int | str | Iterable[int | str])
'Union[int, str, Iterable[int | str]]'

@hhoppe
Copy link
Author

hhoppe commented Aug 3, 2022

Interesting; I can reproduce your results above, but then also:

>>> def f(a: int | str | Iterable[int | str] = 1):
>>>   return a
>>> inspect.formatannotation(f.__annotations__)
"{'a': 'int | str | Iterable[int | str]'}"

The difference might be related to the postponed evaluation of the annotation context?

@hhoppe
Copy link
Author

hhoppe commented Aug 4, 2022

Actually, f.__annotations__ already contains the nice strings:
{'a': 'int | str | Iterable[int | str]'}

I'm curious: what would be the drawbacks of directly using the strings from f.__annotations__ for the pdoc documentation (assuming that the code has enabled from __future__ import annotations)?
It gives the user the choice to refer to local type aliases or to fully qualified module.types, etc.

@mhils
Copy link
Member

mhils commented Aug 4, 2022

The main drawback is that linking becomes much harder (how do we know that Doc is actually pdoc.doc.Doc, but maybe we can do some tricks here similar (but simpler) to 1a in the other issue. I'll take a look! :)

@mhils
Copy link
Member

mhils commented Aug 12, 2022

For the record, the problem here is that typing does not perfectly interoperate with the new types.UnionType (on which the | syntax is based). The following works as expected:

from collections.abc import Iterable  # not from typing!

def function2(value: int | str | Iterable[int | str] = 1) -> None:
  pass

@hhoppe
Copy link
Author

hhoppe commented Aug 12, 2022

Amazing that you found this!
I just confirmed on my code that this issue goes away when migrating to using collections.abc.

It does make that pdoc documentation longer, e.g. collections.abc.Iterable[...] so some control over verbosity of type expansion (#420) would still be handy.

@hhoppe hhoppe closed this as completed Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants