eng: add option to put docstrings on model attributes BNCH-114718

[eli-bl]

This PR is for the Benchling fork. There is an equivalent upstream PR, openapi-generators#1190 - see the description there for more details.

The implementation here is exactly equivalent to the upstream PR, so will not change if that one is accepted. There is a difference in the test code: this PR uses the "functional tests" mechanism that exists on our fork, rather than the "golden record" type of test which is the only way to test it upstream.

Why we need this

The tool's current behavior is to make a docstring for each model class that combines the schema-level description with a list of all the properties and their types and descriptions. It ends up looking like this:

classBenchlingAlignedSequence""" An aligned sequence within a NucleotideAlignment Attributes: field_typename (Union[Unset, str]): bases (Union[None, Unset, str]): The bases of the Aligned Sequence, including gaps. name (Union[None, Unset, str]): The name of the Aligned Sequence. pairwise_identity (Union[None, Unset, float]): Fraction of bases between trimStart and trimEnd that match the template bases. Only present for Template Alignments; Will be empty for Consensus Alignments. """field_typename: Union[Unset, str] bases: Union[None, Unset, str] name: Union[None, Unset, str] pairwise_identity: Union[None, Unset, float]

That's nice enough if you're just looking directly at the source code— but it's pretty bad if you plan to use a documentation tool like Sphinx, as we do. The use of line breaks and indents in the "Attributes:" section confuses these tools, if they try to interpret it as any kind of markup— it's not valid in Sphinx's .rst format, and it's not valid in Markdown either. So you end up seeing something like this:

Plus, as you can see in the code example, putting the type signatures of the attributes into the docstring is redundant: they already have type hints in the code, which documentation tools and IDEs already know how to parse.

So, by using the new option in this PR, we can make the code look like this instead:

classBenchlingAlignedSequence""" An aligned sequence within a NucleotideAlignment """field_typename: Union[Unset, str] bases: Union[None, Unset, str] """The bases of the Aligned Sequence, including gaps."""name: Union[None, Unset, str] """The name of the Aligned Sequence."""pairwise_identity: Union[None, Unset, float] """Fraction of bases between trimStart and trimEnd that match the template bases. Only present for Template Alignments; Will be empty for Consensus Alignments."""

This renders nicely in Sphinx, and the attribute descriptions also show up if you hover over an attribute in VS Code.

(The way we used to deal with this in our old SDK was somewhat different: we had heavily customized the generator's template for model classes, so that instead of just declaring attributes and letting attrs auto-generate everything, we were creating custom getter methods and putting docstrings on those. We'd like to avoid doing anything like that in the future; making custom versions of complex generator templates made our code very hard to maintain.)

[eli-bl]

This makes the openapi-python-client config options available as a config variable in the Jinja templates.

[eli-bl]

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.

[damola-benchling]

LGTM.

I think the sample in the unit test should be updated because it's a critical reference for the next person that might work on this

[eli-bl]

The maintainer pointed out on the upstream PR that there was a similar docstring issue in the generated client classes. So I updated this template in the upstream PR & also in this one. As with my other changes, these result in exactly the same code if you do not have the new option enabled - the proof of that is that I didn't need to make any changes in the "golden record" code under end_to_end_tests.

[eli-bl]

You can see what the generated client class looks like when the new option is set here - because, in the upstream PR, I did include the new feature in the "golden record"-based tests (since there are no functional tests on that branch).