Make better docstrings (#1012)

* Improve docstring rendering

Render docstrings from markdown to rest, and change sphinx output to
include init docs.

* More docs changes

* pre-commit fixes

* Try to improve sphinx cache

* Fix restore keys

* Use repo commit as key

* Try to download site

* Doesn't work

* Rebase

* Remove duplicated model docs

* Escape line breaks

* Remove nested blocks

* Rebase

* Rebase

Co-authored-by: ci.datadog-api-spec <[email protected]>

Author: therve

.generator/poetry.lock

@@ -14,6 +14,7 @@ jinja2 = "3.0.3"
 pytest = "^7.1.1"
 pytest-bdd = "^5.0.0"
 python-dateutil = "2.8.2"
+m2r2 = "0.3.2"
 
 [tool.poetry.dev-dependencies]
 

.generator/pyproject.toml

@@ -36,6 +36,7 @@ def cli(specs, output):
     env.filters["parameters"] = openapi.parameters
     env.filters["return_type"] = openapi.return_type
     env.filters["safe_snake_case"] = openapi.safe_snake_case
+    env.filters["docstring"] = formatter.docstring
 
     env.globals["enumerate"] = enumerate
     env.globals["package"] = PACKAGE_NAME
@@ -112,12 +113,14 @@ def cli(specs, output):
         with models_path.open("w") as fp:
             fp.write(models_j2.render(models=sorted(models)))
 
+        tags_by_name = {tag["name"]: tag for tag in spec["tags"]}
+
         for name, operations in apis.items():
             filename = openapi.safe_snake_case(name) + "_api.py"
             api_path = package / "api" / filename
             api_path.parent.mkdir(parents=True, exist_ok=True)
             with api_path.open("w") as fp:
-                fp.write(api_j2.render(name=name, operations=operations))
+                fp.write(api_j2.render(name=name, operations=operations, description=tags_by_name[name]["description"]))
 
         api_init_path = package / "api" / "__init__.py"
         with api_init_path.open("w") as fp:

.generator/src/generator/cli.py

@@ -2,6 +2,8 @@
 import keyword
 import re
 
+import m2r2
+
 KEYWORDS = set(keyword.kwlist)
 KEYWORDS.add("property")
 
@@ -53,3 +55,17 @@ def get_name(schema):
 
 def attribute_path(attribute):
     return ".".join(attribute_name(a) for a in attribute.split("."))
+
+
+class CustomRenderer(m2r2.RestRenderer):
+    def double_emphasis(self, text):
+        if "``" in text:
+            text = text.replace("\ ``", "").replace("``\ ", "")
+        return super().double_emphasis(text)
+
+    def header(self, text, level, raw=None):
+        return "\n{}\n".format(self.double_emphasis(text))
+
+
+def docstring(text):
+    return m2r2.convert(text.replace("\\n", "\\\\n"), renderer=CustomRenderer())[1:-1]

.generator/src/generator/formatter.py

@@ -16,7 +16,9 @@ from {{ package }}.{{ version }}.model.{{ model|safe_snake_case }} import {{ mod
 
 {% set classname = name.replace(" ", "") + "Api" %}
 class {{ classname }}:
-
+    """
+    {{ description|docstring|indent(4) }}
+    """
     def __init__(self, api_client=None):
         if api_client is None:
             api_client = ApiClient()
@@ -135,7 +137,7 @@ class {{ classname }}:
     def {{ operation.operationId|safe_snake_case }}(self, {% for name, parameter in operation|parameters if parameter.required %}{{name|attribute_name}}, {% endfor %}**kwargs):
         """{{ operation.summary|indent(8) }}.
 {% if operation.description %}
-        {{ operation.description.replace("\\", "\\\\")|indent(8) }}
+        {{ operation.description|docstring|indent(8) }}
 {% endif %}
         This method makes a synchronous HTTP request by default. To make an
         asynchronous HTTP request, please pass async_req=True.
@@ -147,12 +149,12 @@ class {{ classname }}:
 
 {%- for name, parameter in operation|parameters if parameter.required %}
 {%- if parameter.description %}
-        :param {{ name|attribute_name }}: {{ parameter.description|indent(12) }}{% if parameter.default %} Defaults to {{ parameter.default }}.{% endif %}{% endif %}
+        :param {{ name|attribute_name }}: {{ parameter.description|docstring|indent(12) }}{% if parameter.default %} Defaults to {{ parameter.default }}.{% endif %}{% endif %}
         :type {{ name|attribute_name }}: {{ get_type_for_parameter(parameter) }}
 {%- endfor %}
 {%- for name, parameter in operation|parameters if not parameter.required %}
 {%- if parameter.description %}
-        :param {{ name|attribute_name }}: {{ parameter.description|indent(12) }}{%- if parameter.default %} If omitted the server will use the default value of {{ parameter.default }}.{% endif %}{% endif %}
+        :param {{ name|attribute_name }}: {{ parameter.description|docstring|indent(12) }}{%- if parameter.default %} If omitted the server will use the default value of {{ parameter.default }}.{% endif %}{% endif %}
         :type {{ name|attribute_name }}: {{ get_type_for_parameter(parameter) }}, optional
 {%- endfor %}
         :param _return_http_data_only: Response data without head status
@@ -197,12 +199,12 @@ class {{ classname }}:
 {# keep new line #}
 {%- for name, parameter in operation|parameters if parameter.required %}
 {%- if parameter.description %}
-        :param {{ name|attribute_name }}: {{ parameter.description|indent(12) }}{% if parameter.default %} Defaults to {{ parameter.default }}.{% endif %}{% endif %}
+        :param {{ name|attribute_name }}: {{ parameter.description|docstring|indent(12) }}{% if parameter.default %} Defaults to {{ parameter.default }}.{% endif %}{% endif %}
         :type {{ name|attribute_name }}: {{ get_type_for_parameter(parameter) }}
 {%- endfor %}
 {%- for name, parameter in operation|parameters if not parameter.required %}
 {%- if parameter.description %}
-        :param {{ name|attribute_name }}: {{ parameter.description|indent(12) }}{%- if parameter.default %} If omitted the server will use the default value of {{ parameter.default }}.{% endif %}{% endif %}
+        :param {{ name|attribute_name }}: {{ parameter.description|docstring|indent(12) }}{%- if parameter.default %} If omitted the server will use the default value of {{ parameter.default }}.{% endif %}{% endif %}
         :type {{ name|attribute_name }}: {{ get_type_for_parameter(parameter) }}, optional
 {%- endfor %}
         :param _request_timeout: Timeout setting for this request. If one

.generator/src/generator/templates/api.j2

@@ -110,10 +110,10 @@ class {{ name }}(ModelNormal):
         {%- if attr in model.get("required", []) and (definition.get("default") == None or definition.enum) %} {{ attr|attribute_name }},{% endif %}
 {%- endfor %} *args, **kwargs):
         """
-        {{ model.description.replace("\\", "\\\\")|indent(8) }}
+        {{ model.description|docstring|indent(8) }}
 {%- for attr, definition in model.get("properties", {}).items() %}
 {# keep new line #}
-        :param {{ attr|attribute_name }}: {{ definition.description|indent(12) }}
+        :param {{ attr|attribute_name }}: {{ definition.description|docstring|indent(12) }}
         :type {{ attr|attribute_name }}: {{ get_type_for_attribute(model, attr, current_name=name) }}{% if definition.nullable %}, none_type{% endif %}{% if attr not in model.get("required", []) %}, optional{% endif %}
 {%- endfor %}
         """

.generator/src/generator/templates/model_generic.j2

@@ -17,7 +17,7 @@ class {{ name }}(ModelComposed):
 
     def __init__(self, *args, **kwargs):
         """
-        {{ model.description|indent(8) }}
+        {{ model.description|docstring|indent(8) }}
 {%- for attr, definition, schema in get_oneof_parameters(model) %}
 {# keep new line #}
         :param {{ attr|attribute_name }}: {{ definition.description|indent(12) }}

.generator/src/generator/templates/model_oneof.j2

@@ -64,11 +64,13 @@ class {{ name }}(ModelSimple):
 
     def __init__(self, *args, **kwargs):
         """
-        {{ model.description|indent(8) }}
+        {{ model.description|docstring|indent(8) }}
 
         Note that value can be passed either in args or in kwargs, but not in both.
-
-        :param value: {{ model.description }}{%- if "default" in model and model.default != None %} If omitted defaults to "{{ model.default }}".{%- endif %}
+{# empty line #}
+{%- if "default" in model and model.default != None %}
+        :param value: If omitted defaults to "{{ model.default }}".
+{%- endif %}
 {%- if ref %}
         :type value: [{{ ref }}]
 {%- else %}

.generator/src/generator/templates/model_simple.j2

@@ -29,11 +29,21 @@ jobs:
       - name: Install tox
         run: pip install tox
 
+      - name: Set spec commit
+        id: set_commit
+        run: |
+          content=`cat .apigentools-info`
+          content="${content//'%'/'%25'}"
+          content="${content//$'\n'/'%0A'}"
+          content="${content//$'\r'/'%0D'}"
+          echo "::set-output name=spec_json::$content"
+
       - name: Cache sphinx
-        uses: actions/cache@v2
+        uses: actions/cache@v3
         with:
           path: docs/.sphinx
-          key: sphinx
+          key: sphinx-${{ fromJson(steps.set_commit.outputs.spec_json).spec_versions.v2.spec_repo_commit }}
+          restore-keys: sphinx-
 
       - name: Build documentation
         run: tox -e docs

.github/workflows/docs.yml

@@ -24,7 +24,7 @@ repos:
     name: Generate API docs
     stages: [manual]
     language: python
-    entry: sphinx-apidoc --output-dir docs --force --no-toc --templatedir docs/_templates src/datadog_api_client *apis *models *version.py
+    entry: bash -c "SPHINX_APIDOC_OPTIONS=members,show-inheritance sphinx-apidoc --output-dir docs --force --no-toc --templatedir docs/_templates src/datadog_api_client *apis *models *version.py"
     pass_filenames: false
     additional_dependencies:
       - Sphinx