Improve sphinx documentation rendering to use autodoc type hints for params and not in signatures #928
+134
−1,086
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These are needed in order for doc references to these values to work.
Sphinx will default to picking up the repr of an object, which for MISSING turns into `<globus_sdk.MISSING>`, and makes the signature lines bulky. Attach a listener for the autodoc signature event which rewrites any instance of this to the simple string `MISSING`.
This allows sphinx autodoc typehints to populate the type values. The only exception is `get_jwk` because it uses type overloads. In that one case, the explicit type documentation must be retained to instruct Sphinx on how to document it.
- remove `:type ...:` and `:rtype ...:` documentation - set `autodoc_typehints_description_target="documented_params"` to improve client class initializer documentation - reorder unions of `MissingType|Literal[...]` in order to avoid a sphinx bug which appears to incorrectly render a Literal union in which the Literal is the left-hand side - alter ScopeCollectionType to allow autodoc inspection Regarding ScopeCollectionType, the basic change is that we now allow `globus_sdk._types` to import `MutableScope` at runtime and move the deferred type-checking imports into `globus_sdk.scopes`. As a result, the runtime-visible value contains the proper `MutableScope` object, not a string forward reference (which sphinx doesn't know how to resolve). This slight rearrangement basically makes sphinx work. Additionally, since we were changing the type, remove two elements of the union which are redundant. Most likely, this had previously been written with `list`, which is invariant, and was not correctly fixed when it was widened to `Iterable`.
- remove `:type:` docs - move *non typing* information from the `:type:` area to `:param:` as appropriate, always as a simple append
- remove all uses of `:type:` docs - add `ArrayResponse` (which was missing) so that doc linking works for rtype and docs are more complete - in the case of `GroupPolicies`, special handling produces UnionType formatted strings, rather than allowing autodoc logic to run For whatever reason, autodoc does not correctly process a union when it gets the union type via the signature of `__init__`, and can't resolve the type alias there in the same way that it does for methods. To work around this for now, the docstring of the class is updated to be a non-interpolated format-string, which is then converted at runtime using `.format()` via a decorator. This keeps the string definition of these unions local to the place where they are defined and used.
Also fix a minor error in the class docstring for TransferClient (missing newline before list).
These are not all publicly documented, but they will use sphinx autodoc type hints if they are. Document them internally in consistent style.
Paging classes documented the type of the argument tuple correctly, but then provided an annotation for it as `list[Any]`. Fix this by making it properly a tuple (which is what it was always intended to be). This constitutes a minor change at runtime from a list which is never modified to a tuple, but most importantly fixes the incorrect type annotations.
sirosen
requested review from
ada-globus,
derek-globus,
kurtmckee and
aaschaer
as code owners
kurtmckee
approved these changes
Dec 20, 2023
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This includes some minor additional fixes but is mostly a long procedural change from using explicit
:type:documentation to using autodoc type hints as much as possible.This reduces a great deal of duplication and visual noise, making signatures significantly more readable in our rendered docs.
In a couple of cases, we need to get slightly "clever" in order for the hints to render well.
One of the more obvious ones of these is customized rendering of
globus_sdk.MISSINGas the bare stringMISSINGin signatures.That makes
MISSINGusage look highly ergonomic in signatures.In the case of Groups, there was one case of a class,
GroupPolicies, whose docstring did not render correctly under autodoc.A decorator, local to the module where
GroupPoliciesis defined, makes a modification to the__doc__of that class to give sphinx the appropriate values.In
globus_sdk.paging, there was a minor fix as this changeset uncovered a discrepancy between the type annotations and documented type.The documentation was more correct (splatted args are a tuple, not a list), so changes have been made to align with that.
In
AuthClient, there are some unions ofLiteral[...]withMissingTypewhich sphinx only renders correctly ifMissingTypeis the first member of the union, so for simplicitly these are handled by changing the union order to putMissingTypefirst.(This looks to be a sphinx bug, which will need a MRE + bug report.)
Under Flows, several methods had constraints (e.g. string length) documented in the type information.
That doesn't fit within the auspices of the Python type system -- there's no definition for constrained types within the language -- so this information was preserved by moving it to the parameter documentation in each case.
In a few cases,
:rtype:and:type:have been used to explicitly document overrides which replace the autodoc type hint behavior.📚 Documentation preview 📚: https://globus-sdk-python--928.org.readthedocs.build/en/928/
Here's a pair of before/after clips of AuthClient. First of some innocuous methods, then of a pathological one.
Note how the return type is now always annotated. Although things might be slightly longer in the simple cases, in the pathological cases they are dramatically improved.