Extract and re-inject base client sphinx param doc into client classes, for better sphinx doc (#1131)

sirosen · web-flow · commit e0bf525c9dda · 2025-01-30T15:35:43.000-06:00
* Extract and re-inject base client sphinx param doc

Add three helpers to `utils` in order to improve the runtime
docstrings for client classes.

1. `read_sphinx_params` -- a simple `:param:` parser
2. `inject_sphinx_params` -- a `__doc__` rewriter which takes parsed
        params and inserts them in place of a special marker comment
3. `inject_sphinx_params_of` -- a decorator version of these which
        chains together reading the docstring of one object and
        modifying the docstring of another

Then apply these to write

    ```python
    @utils.inject_sphinx_params_of(client.BaseClient)
    class FlowsClient(client.BaseClient):
        ...
    ```

and so forth. This is applied to all client classes except for the
Auth login clients, which do not inherit all of the parameters of
`BaseClient` -- in particular, we do not allow construction of a
login client with an `app`.

`read_sphinx_params` gets a functools cache, but the rest of the work
is still done dynamically, so there is a little bit of repetition.

This results in improved docstrings for sphinx and for runtime usages
like `help(globus_sdk.TransferClient)`. Static analyzers (e.g., IDEs)
will pick up on the original source string, which will show the marker
comment used to drive this behavior:

    ```
    .. globus-sdk-inject-doc-params
    ```

* Convert copy-params to custom sphinx directive

- Remove utility functions for adding parsed `:param:` doc to a
  runtime docstring
- Add a new `CopyParams` sphinx directive registered under the name
  `sdk-sphinx-copy-params`, and use this to implement param
  copy/injection functionality as a documentation-time customization.

There are some particulars about how this looks in the Sphinx/docutils
context which drive the implementation to look a little different from
discussed ideas:

1. The source for doc has to be explicit. `:inherit:` or similar ideas
   about picking up the current class and class hierarchy aren't
   easily implemented in a sphinx plugin. The context of directive
   execution isn't populated with easy values for inspection, and
   finding the "current object being documented" is much more work
   than it is worth. So each usage specifies `BaseClient` as the
   source. (required param)

2. We support added parameters as directive contents. Having a custom
   directive adjacent to a `:param:` list causes docutils errors which
   kill the entire build. Having blank lines between `:param:` lists
   causes multiple `PARAMETERS` blocks to get rendered. So we need
   adjacency but we can't have _literal_ adjacency. If we put params
   into the content area, we can re-emit them from the directive.

2a. Supporting added parameters before and/or after the copied params
    with "&lt;YIELD&gt;". Although not strictly necessary, I've tested to
    ensure we can add params before or after the generated/copied
    params by having a special line in the content of "&lt;YIELD&gt;". The
    first time the directive sees this line, it will stop emitting
    content, emit the copied params, and then resume.

* Minor variable rename + comment per review
diff --git a/changelog.d/20250118_122417_sirosen_better_client_param_doc.rst b/changelog.d/20250118_122417_sirosen_better_client_param_doc.rst
@@ -0,0 +1,5 @@
+Added
+~~~~~
+
+- Most client classes now have their ``__doc__`` attribute modified at runtime
+  to provide better ``help()`` and sphinx documentation. (:pr:`NUMBER`)
diff --git a/src/globus_sdk/_sphinxext.py b/src/globus_sdk/_sphinxext.py
@@ -335,6 +335,38 @@ def gen_rst(self):
         yield ":ref:`how to make paginated calls <making_paginated_calls>`."
 
 
+class CopyParams(AddContentDirective):
+    has_content = True
+    required_arguments = 1
+    optional_arguments = 0
+
+    def gen_rst(self):
+        import globus_sdk
+
+        source_object_name: str = self.arguments[0]
+        path = source_object_name.split(".")
+
+        # traverse `globus_sdk` element by element to find the target
+        source_object = globus_sdk
+        for element in path:
+            source_object = getattr(source_object, element)
+
+        if self.content:
+            content = iter(self.content)
+        else:
+            content = ()
+
+        for line in content:
+            if line.strip() == "<YIELD>":
+                break
+            yield line
+
+        yield from globus_sdk.utils.read_sphinx_params(source_object.__doc__)
+
+        for line in content:
+            yield line
+
+
 def after_autodoc_signature_replace_MISSING_repr(  # pylint: disable=missing-param-doc,missing-type-doc  # noqa: E501
     app,  # pylint: disable=unused-argument
     what,  # pylint: disable=unused-argument
@@ -365,6 +397,7 @@ def setup(app):
     app.add_directive("expandtestfixture", ExpandTestingFixture)
     app.add_directive("extdoclink", ExternalDocLink)
     app.add_directive("paginatedusage", PaginatedUsage)
+    app.add_directive("sdk-sphinx-copy-params", CopyParams)
 
     app.add_role("extdoclink", extdoclink_role)
 
diff --git a/src/globus_sdk/services/auth/client/service_client.py b/src/globus_sdk/services/auth/client/service_client.py
@@ -76,6 +76,8 @@ class AuthClient(client.BaseClient):
     A client for using the
     `Globus Auth API <https://docs.globus.org/api/auth/>`_
 
+    .. sdk-sphinx-copy-params:: BaseClient
+
     This class provides helper methods for most common resources in the
     Auth API, and the common low-level interface from
     :class:`BaseClient <globus_sdk.client.BaseClient>` of ``get``, ``put``,
diff --git a/src/globus_sdk/services/compute/client.py b/src/globus_sdk/services/compute/client.py
@@ -16,6 +16,8 @@ class ComputeClientV2(client.BaseClient):
     r"""
     Client for the Globus Compute API, version 2.
 
+    .. sdk-sphinx-copy-params:: BaseClient
+
     .. automethodlist:: globus_sdk.ComputeClientV2
     """
 
@@ -353,5 +355,7 @@ class ComputeClient(ComputeClientV2):
     Canonical client for the Globus Compute API, with support exclusively for
     API version 2.
 
+    .. sdk-sphinx-copy-params:: BaseClient
+
     .. automethodlist:: globus_sdk.ComputeClient
     """
diff --git a/src/globus_sdk/services/flows/client.py b/src/globus_sdk/services/flows/client.py
@@ -24,6 +24,8 @@ class FlowsClient(client.BaseClient):
     r"""
     Client for the Globus Flows API.
 
+    .. sdk-sphinx-copy-params:: BaseClient
+
     .. automethodlist:: globus_sdk.FlowsClient
     """
 
@@ -833,7 +835,9 @@ class SpecificFlowClient(client.BaseClient):
     Unlike other client types, this must be provided with a specific flow id. All other
         arguments are the same as those for :class:`~globus_sdk.BaseClient`.
 
-    :param flow_id: The generated UUID associated with a flow
+    .. sdk-sphinx-copy-params:: BaseClient
+
+        :param flow_id: The generated UUID associated with a flow
 
     .. automethodlist:: globus_sdk.SpecificFlowClient
     """
diff --git a/src/globus_sdk/services/gcs/client.py b/src/globus_sdk/services/gcs/client.py
@@ -34,7 +34,9 @@ class GCSClient(client.BaseClient):
     Manager. All other arguments are the same as those for
     :class:`~globus_sdk.BaseClient`.
 
-    :param gcs_address: The FQDN (DNS name) or HTTPS URL for the GCS Manager API.
+    .. sdk-sphinx-copy-params:: BaseClient
+
+        :param gcs_address: The FQDN (DNS name) or HTTPS URL for the GCS Manager API.
 
     .. automethodlist:: globus_sdk.GCSClient
     """
diff --git a/src/globus_sdk/services/groups/client.py b/src/globus_sdk/services/groups/client.py
@@ -15,6 +15,8 @@ class GroupsClient(client.BaseClient):
     Client for the
     `Globus Groups API <https://docs.globus.org/api/groups/>`_.
 
+    .. sdk-sphinx-copy-params:: BaseClient
+
     This provides a relatively low level client to public groups API endpoints.
     You may also consider looking at the GroupsManager as a simpler interface
     to more common actions.
diff --git a/src/globus_sdk/services/search/client.py b/src/globus_sdk/services/search/client.py
@@ -18,6 +18,8 @@ class SearchClient(client.BaseClient):
     r"""
     Client for the Globus Search API
 
+    .. sdk-sphinx-copy-params:: BaseClient
+
     This class provides helper methods for most common resources in the
     API, and basic ``get``, ``put``, ``post``, and ``delete`` methods
     from the base client that can be used to access any API resource.
diff --git a/src/globus_sdk/services/timers/client.py b/src/globus_sdk/services/timers/client.py
@@ -23,6 +23,8 @@ class TimersClient(client.BaseClient):
     r"""
     Client for the Globus Timer API.
 
+    .. sdk-sphinx-copy-params:: BaseClient
+
     .. automethodlist:: globus_sdk.TimersClient
     """
 
diff --git a/src/globus_sdk/services/transfer/client.py b/src/globus_sdk/services/transfer/client.py
@@ -46,6 +46,8 @@ class TransferClient(client.BaseClient):
     Client for the
     `Globus Transfer API <https://docs.globus.org/api/transfer/>`_.
 
+    .. sdk-sphinx-copy-params:: BaseClient
+
     This class provides helper methods for most common resources in the
     REST API, and basic ``get``, ``put``, ``post``, and ``delete`` methods
     from the base rest client that can be used to access any REST resource.
diff --git a/src/globus_sdk/utils.py b/src/globus_sdk/utils.py
@@ -2,10 +2,12 @@
 
 import collections
 import collections.abc
+import functools
 import hashlib
 import os
 import platform
 import sys
+import textwrap
 import typing as t
 import uuid
 from base64 import b64encode
@@ -276,3 +278,48 @@ def classproperty(func: t.Callable[[T], R]) -> _classproperty[T, R]:
     def classproperty(func: t.Callable[[T], R]) -> _classproperty[T, R]:
         # type cast to convert instance method to class method
         return _classproperty(t.cast(t.Callable[[t.Type[T]], R], func))
+
+
+@functools.lru_cache(maxsize=None)
+def read_sphinx_params(docstring: str) -> tuple[str, ...]:
+    """
+    Given a docstring, extract the `:param:` declarations into a tuple of strings.
+
+    :param docstring: The ``__doc__`` to parse, as it appeared on the original object
+
+    Params start with `:param ...` and end
+    - at the end of the string
+    - at the next param
+    - when a non-indented, non-param line is found
+
+    Whitespace lines within a param doc are supported.
+
+    All produced param strings are dedented.
+    """
+    docstring = textwrap.dedent(docstring)
+
+    result: list[str] = []
+    current: list[str] = []
+    for line in docstring.splitlines():
+        if not current:
+            if line.startswith(":param"):
+                current = [line]
+            else:
+                continue
+        else:
+            # a new param -- flush the current one and restart
+            if line.startswith(":param"):
+                result.append("\n".join(current).strip())
+                current = [line]
+            # a continuation line for the current param (indented)
+            # or a blank line -- it *could* be a continuation of param doc (include it)
+            elif line != line.lstrip() or not line:
+                current.append(line)
+            # otherwise this is a populated line, not indented, and without a `:param`
+            # start -- stop looking for more param doc
+            else:
+                break
+    if current:
+        result.append("\n".join(current).strip())
+
+    return tuple(result)
diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py
@@ -1,3 +1,4 @@
+import textwrap
 import uuid
 
 import pytest
@@ -117,3 +118,51 @@ def y(self_or_cls):
 )
 def test_safe_strseq_iter(value, expected_result):
     assert list(utils.safe_strseq_iter(value)) == expected_result
+
+
+def test_read_sphinx_params():
+    docstring = """
+    preamble
+
+    :param param1: some doc on one line
+    :param param2: other doc
+        spanning multiple lines
+    :param param3: a doc
+        spanning
+        many
+        lines
+    :param param4: a doc
+        spanning lines
+
+        with a break in the middle ^
+    :param param5: another
+
+
+    :param param6: and a final one after some whitespace
+
+    epilogue
+    """
+    params = utils.read_sphinx_params(docstring)
+    assert len(params) == 6
+    assert params[0] == ":param param1: some doc on one line"
+    assert params[1] == ":param param2: other doc\n    spanning multiple lines"
+    assert params[2] == textwrap.dedent(
+        """\
+        :param param3: a doc
+            spanning
+            many
+            lines"""
+    )
+    assert params[3] == textwrap.dedent(
+        """\
+        :param param4: a doc
+            spanning lines
+
+            with a break in the middle ^"""
+    )
+    assert params[4] == ":param param5: another"
+    assert params[5] == ":param param6: and a final one after some whitespace"
+
+    # clear the cache after the test
+    # not essential, but reduces the risk that this impacts some future test
+    utils.read_sphinx_params.cache_clear()

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +Added
 +~~~~~
++
 +- Most client classes now have their ``__doc__`` attribute modified at runtime
 +  to provide better ``help()`` and sphinx documentation. (:pr:`NUMBER`)