From 05e945429cbf840535e4671d0da5712bc9d4f2b9 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Thu, 22 Aug 2019 15:34:49 +0200 Subject: [PATCH] documentation/python: implement documenting function params and return. Did not expect I would need to patch Docutils for this because nobody had this use case since its initial commit in 2002. How the heck is Sphinx doing this, are they replacing Docutils internals too?! --- doc/documentation/python.rst | 3 + doc/plugins/sphinx.rst | 32 +++++- documentation/python.py | 100 +++++++++++++++++- .../templates/python/details-function.html | 4 +- .../test_python/content/content.Class.html | 26 +++++ .../test_python/content/content.html | 81 +++++++++++--- .../test_python/content/content/__init__.py | 10 +- documentation/test_python/content/docs.rst | 20 +++- plugins/m/sphinx.py | 18 +++- 9 files changed, 266 insertions(+), 28 deletions(-) diff --git a/doc/documentation/python.rst b/doc/documentation/python.rst index 276fb8ad..4b2306e4 100644 --- a/doc/documentation/python.rst +++ b/doc/documentation/python.rst @@ -1046,6 +1046,8 @@ Property Description arguments). Set to :py:`False` when wrapping on multiple lines would only occupy too much vertical space. +:py:`function.has_param_details` If the function parameters are documented +:py:`function.return_value` Return value documentation. Can be empty. :py:`function.has_details` If there is enough content for the full description block [2]_ :py:`function.is_classmethod` Set to :py:`True` if the function is @@ -1070,6 +1072,7 @@ Property Description :py:`param.kind` Parameter kind, a string equivalent to one of the `inspect.Parameter.kind `_ values +:py:`param.content` Detailed documentation, if any =========================== =================================================== In some cases (for example in case of native APIs), the parameters can't be diff --git a/doc/plugins/sphinx.rst b/doc/plugins/sphinx.rst index 7dd5c16a..0b65ed0c 100644 --- a/doc/plugins/sphinx.rst +++ b/doc/plugins/sphinx.rst @@ -68,11 +68,13 @@ List the plugin in your :py:`PLUGINS`. The :rst:`.. py:module::`, :rst:`.. py:class::`, :rst:`.. py:enum::`, :rst:`.. py:function::`, :rst:`.. py:property::` and :rst:`.. py:data::` directives provide a way to supply module, class, enum, function / method, -property and data documentation content. Directive option is the name to -document, directive contents are the actual contents; in addition the -:py:`:summary:` option can override the docstring extracted using inspection. -No restrictions are made on the contents, it's possible to make use of any -additional plugins in the markup. Example: +property and data documentation content. + +Directive option is the name to document, directive contents are the actual +contents; in addition all the directives have the :py:`:summary:` option that +can override the docstring extracted using inspection. No restrictions are made +on the contents, it's also possible to make use of any additional plugins in +the markup. Example: .. code:: rst @@ -105,3 +107,23 @@ actual rendered docs. exist (i.e., accessible via inspection) in given module. If given name doesn't exist, a warning will be printed during processing and the documentation ignored. + +The :rst:`.. py:function::` directive supports additional options --- +:py:`:param :` for documenting parameters and :py:`:return:` for +documenting the return value. It's allowed to have either none or all +parameters documented (the ``self`` parameter can be omitted), having them +documented only partially or documenting parameters that are not present in the +function signature will cause a warning. Example: + +.. code:: rst + + .. py:function:: mymodule.MyContainer.add + :param key: Key to add + :param value: Corresponding value + :param overwrite_existing: Overwrite existing value if already present + in the container + :return: The inserted tuple or the existing + key/value pair in case ``overwrite_existing`` is not set + + Add a key/value pair to the container, optionally overwriting the + previous value. diff --git a/documentation/python.py b/documentation/python.py index 169b2fa0..9ce61e6b 100755 --- a/documentation/python.py +++ b/documentation/python.py @@ -27,6 +27,7 @@ import argparse import copy import docutils +import docutils.utils import enum import urllib.parse import hashlib @@ -1125,6 +1126,7 @@ def extract_function_doc(state: State, parent, entry: Empty) -> List[Any]: overloads += [out] + # TODO: assign docs and particular param docs to overloads return overloads # Sane introspection path for non-pybind11 code @@ -1172,6 +1174,33 @@ def extract_function_doc(state: State, parent, entry: Empty) -> List[Any]: param.kind = str(i.kind) out.params += [param] + # Get docs for each param and for the return value + path_str = '.'.join(entry.path) + if path_str in state.function_docs: + # Having no parameters documented is okay, having self + # undocumented as well. But having the rest documented only + # partially isn't okay. + if state.function_docs[path_str]['params']: + param_docs = state.function_docs[path_str]['params'] + used_params = set() + for param in out.params: + if param.name not in param_docs: + if param.name != 'self': + logging.warning("%s() parameter %s is not documented", path_str, param.name) + continue + param.content = render_inline_rst(state, param_docs[param.name]) + used_params.add(param.name) + out.has_param_details = True + out.has_details = True + # Having unused param docs isn't okay either + for name, _ in param_docs.items(): + if name not in used_params: + logging.warning("%s() documents parameter %s, which isn't in the signature", path_str, name) + + if state.function_docs[path_str]['return']: + out.return_value = render_inline_rst(state, state.function_docs[path_str]['return']) + out.has_details = True + # In CPython, some builtin functions (such as math.log) do not provide # metadata about their arguments. Source: # https://docs.python.org/3/library/inspect.html#inspect.signature @@ -1589,6 +1618,62 @@ class _SaneInlineHtmlTranslator(m.htmlsanity.SaneHtmlTranslator): def render_inline_rst(state: State, source): return publish_rst(state, source, translator_class=_SaneInlineHtmlTranslator).writer.parts.get('body').rstrip() +# Copy of docutils.utils.extract_options which doesn't throw BadOptionError on +# multi-word field names but instead turns the body into a tuple containing the +# extra arguments as a prefix and the original data as a suffix. The original +# restriction goes back to a nondescript "updated" commit from 2002, with no +# change of this behavior since: +# https://github.com/docutils-mirror/docutils/commit/508483835d95632efb5dd6b69c444a956d0fb7df +def _docutils_extract_options(field_list): + option_list = [] + for field in field_list: + field_name_parts = field[0].astext().split() + name = str(field_name_parts[0].lower()) + body = field[1] + if len(body) == 0: + data = None + elif len(body) > 1 or not isinstance(body[0], docutils.nodes.paragraph) \ + or len(body[0]) != 1 or not isinstance(body[0][0], docutils.nodes.Text): + raise docutils.utils.BadOptionDataError( + 'extension option field body may contain\n' + 'a single paragraph only (option "%s")' % name) + else: + data = body[0][0].astext() + if len(field_name_parts) > 1: + # Supporting just one argument, don't need more right now (and + # allowing any number would make checks on the directive side too + # complicated) + if len(field_name_parts) != 2: raise docutils.utils.BadOptionError( + 'extension option field name may contain either one or two words') + data = tuple(field_name_parts[1:] + [data]) + option_list.append((name, data)) + return option_list + +# ... and allowing duplicate options as well. This restriction goes back to the +# initial commit in 2002. Here for duplicate options we expect the converter to +# give us a list and we merge those lists; if not, we throw +# DuplicateOptionError as in the original code. +def _docutils_assemble_option_dict(option_list, options_spec): + options = {} + for name, value in option_list: + convertor = options_spec[name] # raises KeyError if unknown + if convertor is None: + raise KeyError(name) # or if explicitly disabled + try: + converted = convertor(value) + except (ValueError, TypeError) as detail: + raise detail.__class__('(option: "%s"; value: %r)\n%s' + % (name, value, ' '.join(detail.args))) + if name in options: + if isinstance(converted, list): + assert isinstance(options[name], list) and not isinstance(options[name], tuple) + options[name] += converted + else: + raise docutils.utils.DuplicateOptionError('duplicate non-list option "%s"' % name) + else: + options[name] = converted + return options + def render_doc(state: State, filename): logging.debug("parsing docs from %s", filename) @@ -1596,8 +1681,19 @@ def render_doc(state: State, filename): # these functions are not generating any pages # Render the file. The directives should take care of everything, so just - # discard the output afterwards. - with open(filename, 'r') as f: publish_rst(state, f.read()) + # discard the output afterwards. Some directives (such as py:function) have + # multi-word field names and can be duplicated, so we have to patch the + # option extractor to allow that. See _docutils_extract_options and + # _docutils_assemble_option_dict above for details. + with open(filename, 'r') as f: + prev_extract_options = docutils.utils.extract_options + prev_assemble_option_dict = docutils.utils.assemble_option_dict + docutils.utils.extract_options = _docutils_extract_options + docutils.utils.assemble_option_dict = _docutils_assemble_option_dict + + publish_rst(state, f.read()) + docutils.utils.extract_options = prev_extract_options + docutils.utils.assemble_option_dict = prev_assemble_option_dict def render_page(state: State, path, input_filename, env): filename, url = state.config['URL_FORMATTER'](EntryType.PAGE, path) diff --git a/documentation/templates/python/details-function.html b/documentation/templates/python/details-function.html index b78ae71f..15068646 100644 --- a/documentation/templates/python/details-function.html +++ b/documentation/templates/python/details-function.html @@ -14,10 +14,12 @@ {% for param in function.params %} + {% if loop.index != 1 or param.name != 'self' or param.content %} {{ param.name }} - {{ param.description }} + {{ param.content }} + {% endif %} {% endfor %} {% endif %} diff --git a/documentation/test_python/content/content.Class.html b/documentation/test_python/content/content.Class.html index 66eb5822..26d9b056 100644 --- a/documentation/test_python/content/content.Class.html +++ b/documentation/test_python/content/content.Class.html @@ -67,6 +67,10 @@ indented.

This overwrites the docstring for content.Class.method, but doesn't add any detailed block.
+
+ def method_param_docs(self, a, b) +
+
This method gets its params except self documented
def method_with_details(self)
@@ -124,6 +128,28 @@ but doesn't add any detailed block.

This function is a static method

The staticmethod should be shown here.

+ +
+

+ def content.Class.method_param_docs(self, a, b) +

+

This method gets its params except self documented

+ + + + + + + + + + + + + + +
Parameters
aThe first parameter
bThe second parameter
+

The self isn't documented and thus also not included in the list.

diff --git a/documentation/test_python/content/content.html b/documentation/test_python/content/content.html index 6fc4bcc8..d5f405c4 100644 --- a/documentation/test_python/content/content.html +++ b/documentation/test_python/content/content.html @@ -74,12 +74,6 @@ doesn't add any detailed block.

Functions

-
- def annotations(a: int, - b, - c: float) -> str -
-
No annotations shown for this
def foo(a, b)
@@ -93,6 +87,16 @@ doesn't add any detailed block. def function_with_summary()
This function has summary from the docstring
+
+ def param_docs(a: int, + b, + c: float) -> str +
+
Detailed param docs and annotations
+
+ def param_docs_wrong(a, b) +
+
Should give warnings
@@ -139,15 +143,6 @@ doesn't add any detailed block.

Function documentation

-
-

- def content.annotations(a: int, - b, - c: float) -> str -

-

No annotations shown for this

-

Type annotations in detailed docs.

-

def content.foo_with_details(a, b) @@ -163,6 +158,62 @@ Detailed docs for this function

This function has summary from the docstring

This function has external details but summary from the docstring.

+
+

+ def content.param_docs(a: int, + b, + c: float) -> str +

+

Detailed param docs and annotations

+ + + + + + + + + + + + + + + + + + + + + + + + +
Parameters
aFirst parameter
bThe second one
cAnd a float
ReturnsString, of course, it's all stringly typed
+

Type annotations and param list in detailed docs.

+
+
+

+ def content.param_docs_wrong(a, b) +

+

Should give warnings

+ + + + + + + + + + + + + + +
Parameters
aFirst
b
+

The b is not documented, while c isn't in the signature.

+

Data documentation

diff --git a/documentation/test_python/content/content/__init__.py b/documentation/test_python/content/content/__init__.py index f48c850a..be50c537 100644 --- a/documentation/test_python/content/content/__init__.py +++ b/documentation/test_python/content/content/__init__.py @@ -23,6 +23,9 @@ class Class: def method_with_details(self): pass + def method_param_docs(self, a, b): + """This method gets its params except self documented""" + @property def a_property(self): """This summary is not shown either""" @@ -60,8 +63,11 @@ def foo_with_details(a, b): def function_with_summary(): """This function has summary from the docstring""" -def annotations(a: int, b, c: float) -> str: - """No annotations shown for this""" +def param_docs(a: int, b, c: float) -> str: + """Detailed param docs and annotations""" + +def param_docs_wrong(a, b): + """Should give warnings""" CONSTANT: float = 3.14 diff --git a/documentation/test_python/content/docs.rst b/documentation/test_python/content/docs.rst index bef6a8e8..5bf105a5 100644 --- a/documentation/test_python/content/docs.rst +++ b/documentation/test_python/content/docs.rst @@ -39,6 +39,12 @@ This one has a detailed block without any summary. +.. py:function:: content.Class.method_param_docs + :param a: The first parameter + :param b: The second parameter + + The ``self`` isn't documented and thus also not included in the list. + .. py:property:: content.Class.a_property :summary: This overwrites the docstring for ``content.Class.a_property``, but doesn't add any detailed block. @@ -83,9 +89,19 @@ This function has external details but summary from the docstring. -.. py:function:: content.annotations +.. py:function:: content.param_docs + :param a: First parameter + :param b: The second one + :param c: And a ``float`` + :return: String, of course, it's all *stringly* typed + + Type annotations and param list in detailed docs. + +.. py:function:: content.param_docs_wrong + :param a: First + :param c: Third - Type annotations in detailed docs. + The ``b`` is not documented, while ``c`` isn't in the signature. .. py:data:: content.CONSTANT :summary: This is finally a docstring for ``content.CONSTANT`` diff --git a/plugins/m/sphinx.py b/plugins/m/sphinx.py index cc11538e..56a594e7 100644 --- a/plugins/m/sphinx.py +++ b/plugins/m/sphinx.py @@ -71,15 +71,31 @@ class PyEnum(rst.Directive): } return [] +# List option (allowing multiple arguments). See _docutils_assemble_option_dict +# in python.py for details. +def directives_unchanged_list(argument): + return [directives.unchanged(argument)] + class PyFunction(rst.Directive): final_argument_whitespace = True has_content = True required_arguments = 1 - option_spec = {'summary': directives.unchanged} + option_spec = {'summary': directives.unchanged, + 'param': directives_unchanged_list, + 'return': directives.unchanged} def run(self): + # Check that params are parsed properly, turn them into a dict. This + # will blow up if the param name is not specified. + params = {} + for name, content in self.options.get('param', []): + if name in params: raise KeyError(f"duplicate param {name}") + params[name] = content + function_doc_output[self.arguments[0]] = { 'summary': self.options.get('summary', ''), + 'params': params, + 'return': self.options.get('return'), 'content': '\n'.join(self.content) } return [] -- 2.30.2