beeware / toga

A Python native, OS native GUI toolkit.
https://toga.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
4.19k stars 655 forks source link

Custom type hint doc formatting for `TypeAlias`, `TypeVar` #2645

Open rmartin16 opened 2 weeks ago

rmartin16 commented 2 weeks ago

Changes

This POC is crude but what do you think about the idea?

PR Checklist:

freakboy3742 commented 2 weeks ago

It's a little unwieldy that we need to manually configure each type alias that we want to support, but the I can't argue with the end product - that's pretty much exactly what I had in mind.

The only nitpick I'd have is that the fallback behavior looks like it's currently TypeVar(SomeT) (e.g., the handling of ImageT in ImageView); but if I'm reading the configuration right, it should be possible to return {name of type} as a default?

Even better would be to have a warning/error raised if there's a type alias that is falling back to this default. Having the docs actively complain if we don't document a new type alias will guarantee that we at least describe what the type means (and, in the case of StyleT, will force us to describe what that actually means).

rmartin16 commented 2 weeks ago

The only nitpick I'd have is that the fallback behavior looks like it's currently TypeVar(SomeT) (e.g., the handling of ImageT in ImageView); but if I'm reading the configuration right, it should be possible to return {name of type} as a default?

Even better would be to have a warning/error raised if there's a type alias that is falling back to this default. Having the docs actively complain if we don't document a new type alias will guarantee that we at least describe what the type means (and, in the case of StyleT, will force us to describe what that actually means).

For TypeVars, it should be possible to do this; that is, we could add a if isinstance(annotation, TypeVar) at the end and throw some kind of warning/error. For TypeAliass, the situation is more complicated because sphinx-autodoc-typehints has already resolved the TypeAlias to its actual types by the time this code is running.

rmartin16 commented 1 week ago

13a5556 is an experiment with making each TypeAlias importable at runtime.

Specific to this effort, it makes it much easier to replace the existing type annotations in the docs using these TypeAliases if I can properly import them in to conf.py. This, however, had a lot of interesting side effects...

The current use of typing-extensions requires that all imports from it are done in a if TYPE_CHECKING block. This is because of typing-extensions is only installed with the dev dependencies. Therefore, any TypeAlias must also be defined within the same block. And of course then any use of a TypeAlias in another module requires its import in a if TYPE_CHECKING block.

But we can't do any of this if we want a TypeAlias to be importable at runtime. So, in lieu of promoting typing-extensions to a full dependency, we could, at least, stop annotating type aliases as TypeAlias. The benefit of this annotation is to resolve ambiguities between a regular module global variable and something a type checker should allow for type checking. However, type checkers will assume a module global is a TypeAlias if it's a valid type....and all of our use-cases meet that requirement.

But then...the real fun begins. If you dig in to how sphinx-autodoc-typehints works....it does tremendously more magic than I would have ever imagined. Unlike a full blown type checker, sphinx-autodoc-typehints can't resolve all of the references in each TypeAlias.....but if you import the missing references in to the module where the TyleAlias is defined, the TypeAlias can be fully resolved for the purposes of documentation.

This became more and more of a mess the further I went with it...but I wanted to at least document it...