add option to put docstrings on model attributes #1190
Conversation
|
|
||
| from typer.testing import CliRunner | ||
|
|
||
| from openapi_python_client.cli import app | ||
|
|
||
|
|
||
| def regen_golden_record(): |
There was a problem hiding this comment.
This file had a large amount of copy-paste; most of the regenerate methods are the same except for the file/path names. I've refactored them to use a common method, clarifying what the differences really are.
I verified that this doesn't change the output for the existing end-to-end tests by deleting all the golden record directories, running pdm regen, and seeing that the new files exactly match what's in source control.
eli-bl
force-pushed
the
model-attr-docstrings
branch
from
ec3a812 to
0c73307
Compare
|
|
||
| # Make `config` available in custom templates | ||
|
|
||
| The configuration options object is now exposed as a variable called `config` in Jinja2 templates. |
There was a problem hiding this comment.
This change isn't really specific to the new feature, but I figured it was useful enough to be worth mentioning.
|
@dbanty FYI, this was motivated partly by trying to build some HTML docs from a generated client with Sphinx, and finding that Sphinx really does not like how the current class docstrings are formatted. The format only really works if you view the strings as plain text; if it's interpreted either as Sphinx's default |
|
@dbanty Btw, I updated the PR description to clarify the behavior of attribute docstrings in Python. |
| {# 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) %} |
There was a problem hiding this comment.
The reason I added the omit_if_empty parameter here is to avoid inserting an empty """ """ for the class docstring if the model class doesn't have any description. That wasn't really an issue with the previous behavior, because model classes almost always have properties so there was always a class docstring that included the property descriptions. I am not setting omit_if_empty in the template unless the new docstrings_on_attributes option is enabled, so that I can guarantee that the behavior is 100% unchanged if you don't set the option.
|
|
||
|
|
||
| Attributes: | ||
| raise_on_unexpected_status: Whether or not to raise an errors.UnexpectedStatus if the API returns a |
There was a problem hiding this comment.
@eli-bl do you think it's worth modifying the client classes as well? Do you end up wanting these documented with Sphinx?
There was a problem hiding this comment.
Ah yeah, I hadn't thought of that. I guess it may as well be consistent - will do.
| @@ -0,0 +1,15 @@ | |||
| class_overrides: | |||
There was a problem hiding this comment.
Most of this config is probably not needed with the smaller test case
There was a problem hiding this comment.
Yeah, will remove copypasta
eli-bl
force-pushed
the
model-attr-docstrings
branch
from
80c799d to
e980228
Compare
eli-bl
force-pushed
the
model-attr-docstrings
branch
from
e980228 to
7c3427e
Compare
| 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) }} |
There was a problem hiding this comment.
The word wrap parameter here looks arbitrary, but that happens to be the value that results in exactly the same formatting for this text as before.
Adds a
docstrings_on_attributesoption that causes property descriptions to appear in docstrings on the attributes, instead of in the class docstring. This is a desirable behavior when using documentation generators like Sphinx.This is a non-breaking change; if the option is not set, code generation is exactly the same as before (evidence: the existing golden-record directories have no changes).
Clarification about how docstrings on attributes work
Python does not treat all docstrings the same, in terms of availability at runtime. A docstring on a class or method is available as a
__doc__attribute— but a docstring on an attribute is not; such a string is just thrown away by the interpreter. If classAhas attributeb, thenA.b.__doc__will never be a thing— you would have to definebas a property getter method instead.However, docstrings on attributes are still valid as per PEP 257 as a thing that can be surfaced by other tools. Sphinx will use them when generating documentation, and VS Code will show them as a description in the popup if you hover over an attribute.