mitmproxy / pdoc

API Documentation for Python Projects
https://pdoc.dev
MIT No Attribution
1.95k stars 194 forks source link

Pipe operand for type unions causes type annotation errors #326

Closed bczsalba closed 2 years ago

bczsalba commented 2 years ago

Problem Description

Natively since Python 3.9, but using from __future__ import annotations on some version prior you can use syntax like str | int instead of Union[str, int]. This is supported by type parsers and python alike, but pdoc seems to not like it that much. While the website builds to a certain point, it doesn't complete the full generation.

Steps to reproduce the behavior:

  1. Parse code with type unions using the pipe operand
  2. ???
  3. Profit

System Information

pdoc: 8.1.0
Python: 3.9.9
Platform: macOS-12.0.1-arm64-arm-64bit
mhils commented 2 years ago

Thanks for the report! Would you have an example file that demonstrates the issue?

bczsalba commented 2 years ago

I am currently working on documenting my project, pytermgui, and that's where I found the issue.

On further look however it seems like it generates all the sites just fine, but the main site doesn't recognize any submodules bar one, and that one doesn't seem to be linking anywhere either.

Thank you for your attention!

Edit: Seems like the other sites were just leftover files from a different run, they no longer show up after a directory wipe. Very odd.

mhils commented 2 years ago

On further look however it seems like it generates all the sites just fine, but the main site doesn't recognize any submodules bar one, and that one doesn't seem to be linking anywhere either.

Shot in the dark: Are you setting __all__ in your module? See https://pdoc.dev/docs/pdoc.html#control-what-is-documented, that also applies to submodules. They do not need to be imported, but they need to be in __all__ if you specify __all__. :)

bczsalba commented 2 years ago

I actually was, thank you for that! It's really nice that it follows __all__ that strictly, but I wouldn't have noticed it myself.

For the record, the other issue still occurs, but thank you for "fixing" this for me!

mhils commented 2 years ago

For the record, the other issue still occurs

Could you clarify what the other issue is specifically, and how I can reproduce it? :)

bczsalba commented 2 years ago

Try running pdoc on pytermgui, that's where I get it. I don't have a computer with me at the moment, but since it's a syntax related issue I think it should happen everywhere.

The issue is that pdoc warns that it doesn't understand the type1 | type2 syntax in place of Union[...], and since the library uses it a lot it comes up in basically every line.

It's only the warnings now with the __all__ issue being resolved with your help, but I thought it might be important to know.

bczsalba commented 2 years ago

The exact warning I get is as follows:

/opt/homebrew/lib/python3.9/site-packages/pdoc/doc_types.py:116: UserWarning: Error parsing type annotation MouseTarget | None for pytermgui.widgets.base.Container.handle_mouse: unsupported operand type(s) for |: 'type' and 'NoneType' warnings.warn(f"Error parsing type annotation {t} for {fullname}: {e}")

mhils commented 2 years ago

Interesting, thanks. Curiously it's working fine for me:

$ pip install git+https://github.com/bczsalba/pytermgui.git
Collecting git+https://github.com/bczsalba/pytermgui.git
  Cloning https://github.com/bczsalba/pytermgui.git to /tmp/pip-req-build-tf9ttqn6
  Running command git clone -q https://github.com/bczsalba/pytermgui.git /tmp/pip-req-build-tf9ttqn6
  Resolved https://github.com/bczsalba/pytermgui.git to commit 79186df21d3c4714a90baf638d867cbd1d7cedcf
$ pdoc --version
pdoc: 8.2.0
Python: 3.10.0
Platform: Linux-4.4.0-19041-Microsoft-x86_64-with-glibc2.31
$ pdoc pytermgui.widgets.base
pdoc server ready at http://localhost:8080
Warn: Error parsing type annotation Optional['_IDManager'] for pytermgui.widgets.base.Widget._id_manager. Import of _IDManager failed: name '_IDManager' is not defined (/mnt/c/Users/user/git/pdoc/pdoc/doc_types.py:141)

Result:

image

Could you try to retrace these steps?

(The _IDManager warning is correct as that is indeed unknown in pytermgui.widgets.base.)

mhils commented 2 years ago

Shot in the dark: PEP 604 / Union Types are Python 3.10. You seem to be running 3.9?

bczsalba commented 2 years ago

The _IDManager part is correct, it's a really hacky solution to avoid circular imports.

I still get the same warnings following your exact steps, and yes I am running 3.9. While the Union Types are supposed to be a 3.10 feature, they were backported down until 3.7 using from __future__ import annotations. Without that import Python will complain about the same issue pdoc does, but once included it works.

mhils commented 2 years ago

A side effect of from __future__ import annotations / PEP 563 is that X | Y union types do not raise an immediate error when you add them to your code, but that's only because they are not evaluated and not because the X | Y implementation has been backported. AFAIK — please correct me if I'm wrong — there are no backports for PEP 604, so you do need Python 3.10 for pdoc to work.

The _IDManager part is correct, it's a really hacky solution to avoid circular imports.

FYI, you can add an if TYPE_CHECKING: block so that pdoc can pick up the reference. You also don't need the quotes around _IDManager because of PEP 563. :)

bczsalba commented 2 years ago

That definitely could be, the only reason I know about it being a thing is I wanted to make pytermgui not rely on 3.9 / 3.10, and that seemed like a better way than rewriting all those annotations. Mypy and pylint both support the syntax in this way (as long as the __future__ import is present), so I didn't really think about it that way. :)

If that is the case, however then it's no problem for pdoc not to support it, however is there any way to silence certain warnings? I honestly did not know about _IDManager being a problem until you pointed that out, as I get about 2 terminal heights worth of the same warning. I know I could just use 3.10, so if it's not an already existing feature I don't think it's worth adding for this little functionality.

Thank you for the tip at the end though, to be honest I wrote that part of the code ages ago and added the PEP 563 backport months later, so I never recognized the two being redundant together.

Thanks for your help! I would consider the issue to be resolved, but I'll leave the closing up to you just in case you wanna do something related to this.

:rocket:

mhils commented 2 years ago

Thanks for the kind words! There is no way to silence certain warnings. For pytermgui, I only get a handful of real and easily fixable ones on Python 3.10. On Linux you can pipe stderr through grep, e.g. pdoc [...] 2> >(grep -v IDManager) to ignore IDManager-related warnings. 😉