From 2bf17c24f3464ee8ccf79edda14efc118efe6098 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 10 Jan 2025 10:15:26 -0800 Subject: [PATCH 1/3] eng: add option to put docstrings on model attributes BNCH-114718 --- README.md | 10 +++++ .../test_docstrings.py | 45 ++++++++++++++++++- openapi_python_client/__init__.py | 1 + openapi_python_client/config.py | 3 ++ openapi_python_client/templates/helpers.jinja | 4 +- .../templates/model.py.jinja | 17 +++++-- 6 files changed, 73 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index f5964f3b1..85efbe0c3 100644 --- a/README.md +++ b/README.md @@ -100,6 +100,16 @@ class_overrides: The easiest way to find what needs to be overridden is probably to generate your client and go look at everything in the `models` folder. +### docstrings_on_attributes + +By default, when `openapi-python-client` generates a model class, it includes a list of attributes and their +descriptions in the docstring for the class. If you set this option to `true`, then the attribute descriptions +will be put in docstrings for the attributes themselves, and will not be in the class docstring. + +```yaml +docstrings_on_attributes: true +``` + ### literal_enums By default, `openapi-python-client` generates classes inheriting for `Enum` for enums. It can instead use `Literal` diff --git a/end_to_end_tests/functional_tests/generated_code_execution/test_docstrings.py b/end_to_end_tests/functional_tests/generated_code_execution/test_docstrings.py index afc0dc3c7..299ad1d41 100644 --- a/end_to_end_tests/functional_tests/generated_code_execution/test_docstrings.py +++ b/end_to_end_tests/functional_tests/generated_code_execution/test_docstrings.py @@ -1,9 +1,11 @@ +import re from typing import Any, List from end_to_end_tests.functional_tests.helpers import ( with_generated_code_import, with_generated_client_fixture, ) +from end_to_end_tests.generated_client import GeneratedClientContext class DocstringParser: @@ -36,11 +38,11 @@ def get_section(self, header_line: str) -> List[str]: required: ["reqStr", "undescribedProp"] """) @with_generated_code_import(".models.MyModel") -class TestSchemaDocstrings: +class TestSchemaDocstringsDefaultBehavior: def test_model_description(self, MyModel): assert DocstringParser(MyModel).lines[0] == "I like this type." - def test_model_properties(self, MyModel): + def test_model_properties_in_model_description(self, MyModel): assert set(DocstringParser(MyModel).get_section("Attributes:")) == { "req_str (str): This is necessary.", "opt_str (Union[Unset, str]): This isn't necessary.", @@ -48,6 +50,45 @@ def test_model_properties(self, MyModel): } +@with_generated_client_fixture( +""" +components: + schemas: + MyModel: + description: I like this type. + type: object + properties: + prop1: + type: string + description: This attribute has a description + prop2: + type: string # no description for this one + required: ["prop1", "prop2"] +""", +config="docstrings_on_attributes: true", +) +@with_generated_code_import(".models.MyModel") +class TestSchemaWithDocstringsOnAttributesOption: + def test_model_description_is_entire_docstring(self, MyModel): + assert MyModel.__doc__.strip() == "I like this type." + + def test_attrs_have_docstrings(self, generated_client: GeneratedClientContext): + # A docstring that appears after an attribute is *not* stored in __doc__ anywhere + # by the interpreter, so we can't inspect it that way-- but it's still valid for it + # to appear there, and it will be recognized by documentation tools. So we'll assert + # that these strings appear in the source code. The code should look like this: + # class MyModel: + # """I like this type." + # prop1: str + # """This attribute has a description" + # prop2: str + # + source_file_path = generated_client.output_path / generated_client.base_module / "models" / "my_model.py" + content = source_file_path.read_text() + assert re.search('\n *prop1: *str\n *""" *This attribute has a description *"""\n', content) + assert re.search('\n *prop2: *str\n *[^"]', content) + + @with_generated_client_fixture( """ tags: diff --git a/openapi_python_client/__init__.py b/openapi_python_client/__init__.py index 13ac6d7f4..e42af56f5 100644 --- a/openapi_python_client/__init__.py +++ b/openapi_python_client/__init__.py @@ -95,6 +95,7 @@ def __init__( self.env.filters.update(TEMPLATE_FILTERS) self.env.globals.update( + config=config, utils=utils, python_identifier=lambda x: utils.PythonIdentifier(x, config.field_prefix), class_name=lambda x: utils.ClassName(x, config.field_prefix), diff --git a/openapi_python_client/config.py b/openapi_python_client/config.py index 9cc002d12..1616ac785 100644 --- a/openapi_python_client/config.py +++ b/openapi_python_client/config.py @@ -41,6 +41,7 @@ class ConfigFile(BaseModel): package_version_override: Optional[str] = None use_path_prefixes_for_title_model_names: bool = True post_hooks: Optional[list[str]] = None + docstrings_on_attributes: bool = False field_prefix: str = "field_" generate_all_tags: bool = False http_timeout: int = 5 @@ -70,6 +71,7 @@ class Config: package_version_override: Optional[str] use_path_prefixes_for_title_model_names: bool post_hooks: list[str] + docstrings_on_attributes: bool field_prefix: str generate_all_tags: bool http_timeout: int @@ -111,6 +113,7 @@ def from_sources( package_version_override=config_file.package_version_override, use_path_prefixes_for_title_model_names=config_file.use_path_prefixes_for_title_model_names, post_hooks=post_hooks, + docstrings_on_attributes=config_file.docstrings_on_attributes, field_prefix=config_file.field_prefix, generate_all_tags=config_file.generate_all_tags, http_timeout=config_file.http_timeout, diff --git a/openapi_python_client/templates/helpers.jinja b/openapi_python_client/templates/helpers.jinja index 180613c02..fd5c3ec86 100644 --- a/openapi_python_client/templates/helpers.jinja +++ b/openapi_python_client/templates/helpers.jinja @@ -1,8 +1,10 @@ -{% macro safe_docstring(content) %} +{% macro safe_docstring(content, omit_if_empty=False) %} {# This macro returns the provided content as a docstring, set to a raw string if it contains a backslash #} +{% if (not omit_if_empty) or (content | trim) %} {% if '\\' in content -%} r""" {{ content }} """ {%- else -%} """ {{ content }} """ {%- endif -%} +{% endif %} {% endmacro %} \ No newline at end of file diff --git a/openapi_python_client/templates/model.py.jinja b/openapi_python_client/templates/model.py.jinja index 739f68962..6cd3fca5e 100644 --- a/openapi_python_client/templates/model.py.jinja +++ b/openapi_python_client/templates/model.py.jinja @@ -47,25 +47,34 @@ T = TypeVar("T", bound="{{ class_name }}") {{ model.example | string | wordwrap(112) | indent(12) }} {% endif %} - {% if model.required_properties or model.optional_properties %} + {% if (not config.docstrings_on_attributes) and (model.required_properties or model.optional_properties) %} Attributes: {% for property in model.required_properties + model.optional_properties %} {{ property.to_docstring() | wordwrap(112) | indent(12) }} {% endfor %}{% endif %} {% endmacro %} +{% macro declare_property(property) %} +{%- if config.docstrings_on_attributes and property.description -%} +{{ property.to_string() }} +{{ safe_docstring(property.description, omit_if_empty=True) | wordwrap(112) }} +{%- else -%} +{{ property.to_string() }} +{%- endif -%} +{% endmacro %} + @_attrs_define class {{ class_name }}: - {{ safe_docstring(class_docstring_content(model)) | indent(4) }} + {{ safe_docstring(class_docstring_content(model), omit_if_empty=config.docstrings_on_attributes) | indent(4) }} {% for property in model.required_properties + model.optional_properties %} {% if property.default is none and property.required %} - {{ property.to_string() }} + {{ declare_property(property) | indent(4) }} {% endif %} {% endfor %} {% for property in model.required_properties + model.optional_properties %} {% if property.default is not none or not property.required %} - {{ property.to_string() }} + {{ declare_property(property) | indent(4) }} {% endif %} {% endfor %} {% if model.additional_properties %} From 08a2e3300c1cc14c4986d3d18eabd35e4deb64ca Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 13 Jan 2025 12:05:29 -0800 Subject: [PATCH 2/3] typo in comments --- .../generated_code_execution/test_docstrings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/end_to_end_tests/functional_tests/generated_code_execution/test_docstrings.py b/end_to_end_tests/functional_tests/generated_code_execution/test_docstrings.py index 299ad1d41..54eea955a 100644 --- a/end_to_end_tests/functional_tests/generated_code_execution/test_docstrings.py +++ b/end_to_end_tests/functional_tests/generated_code_execution/test_docstrings.py @@ -78,9 +78,9 @@ def test_attrs_have_docstrings(self, generated_client: GeneratedClientContext): # to appear there, and it will be recognized by documentation tools. So we'll assert # that these strings appear in the source code. The code should look like this: # class MyModel: - # """I like this type." + # """I like this type.""" # prop1: str - # """This attribute has a description" + # """This attribute has a description""" # prop2: str # source_file_path = generated_client.output_path / generated_client.base_module / "models" / "my_model.py" From 4559dc9a9f2c8fa862520bdef92d5ed8c65b3c94 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 13 Jan 2025 12:09:12 -0800 Subject: [PATCH 3/3] also move docstrings to attributes in client classes --- .../templates/client.py.jinja | 51 ++++++++++++++----- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/openapi_python_client/templates/client.py.jinja b/openapi_python_client/templates/client.py.jinja index aee6096e9..cf0301a9a 100644 --- a/openapi_python_client/templates/client.py.jinja +++ b/openapi_python_client/templates/client.py.jinja @@ -5,6 +5,31 @@ from attrs import define, field, evolve import httpx +{% set attrs_info = { + "raise_on_unexpected_status": namespace( + type="bool", + default="field(default=False, kw_only=True)", + docstring="Whether or not to raise an errors.UnexpectedStatus if the API returns a status code" + " that was not documented in the source OpenAPI document. Can also be provided as a keyword" + " argument to the constructor." + ), + "token": namespace(type="str", default="", docstring="The token to use for authentication"), + "prefix": namespace(type="str", default='"Bearer"', docstring="The prefix to use for the Authorization header"), + "auth_header_name": namespace(type="str", default='"Authorization"', docstring="The name of the Authorization header"), +} %} + +{% macro attr_in_class_docstring(name) %} +{{ name }}: {{ attrs_info[name].docstring }} +{%- endmacro %} + +{% macro declare_attr(name) %} +{% set attr = attrs_info[name] %} +{{ name }}: {{ attr.type }}{% if attr.default %} = {{ attr.default }}{% endif %} +{% if attr.docstring and config.docstrings_on_attributes +%} +"""{{ attr.docstring }}""" +{%- endif %} +{% endmacro %} + @define class Client: """A class for keeping track of data related to the API @@ -29,14 +54,14 @@ class Client: ``httpx_args``: A dictionary of additional arguments to be passed to the ``httpx.Client`` and ``httpx.AsyncClient`` constructor. {% endmacro %} {{ httpx_args_docstring() }} +{% if not config.docstrings_on_attributes %} Attributes: - raise_on_unexpected_status: Whether or not to raise an errors.UnexpectedStatus if the API returns a - status code that was not documented in the source OpenAPI document. Can also be provided as a keyword - argument to the constructor. + {{ attr_in_class_docstring("raise_on_unexpected_status") | wordwrap(101) | indent(12) }} +{% endif %} """ {% macro attributes() %} - raise_on_unexpected_status: bool = field(default=False, kw_only=True) + {{ declare_attr("raise_on_unexpected_status") | indent(4) }} _base_url: str = field(alias="base_url") _cookies: dict[str, str] = field(factory=dict, kw_only=True, alias="cookies") _headers: dict[str, str] = field(factory=dict, kw_only=True, alias="headers") @@ -147,20 +172,20 @@ class AuthenticatedClient: """A Client which has been authenticated for use on secured endpoints {{ httpx_args_docstring() }} +{% if not config.docstrings_on_attributes %} Attributes: - raise_on_unexpected_status: Whether or not to raise an errors.UnexpectedStatus if the API returns a - status code that was not documented in the source OpenAPI document. Can also be provided as a keyword - argument to the constructor. - token: The token to use for authentication - prefix: The prefix to use for the Authorization header - auth_header_name: The name of the Authorization header + {{ attr_in_class_docstring("raise_on_unexpected_status") | wordwrap(101) | indent(12) }} + {{ attr_in_class_docstring("token") | indent(8) }} + {{ attr_in_class_docstring("prefix") | indent(8) }} + {{ attr_in_class_docstring("auth_header_name") | indent(8) }} +{% endif %} """ {{ attributes() }} - token: str - prefix: str = "Bearer" - auth_header_name: str = "Authorization" + {{ declare_attr("token") | indent(4) }} + {{ declare_attr("prefix") | indent(4) }} + {{ declare_attr("auth_header_name") | indent(4) }} {{ builders("AuthenticatedClient") }} {{ httpx_stuff("AuthenticatedClient", "self._headers[self.auth_header_name] = f\"{self.prefix} {self.token}\" if self.prefix else self.token") }}