quicktype icon indicating copy to clipboard operation
quicktype copied to clipboard

Python docstrings are misplaced

Open seiltingcr opened this issue 4 years ago • 5 comments

The python code generator for 3.6 and 3.7 puts the property descriptions as docstrings before the respective properties in the classes. This is incorrect because the first docstring after the class name is applied to the class, and all others disappear at runtime.

This causes the classes to be documented with the descriptions of their first properties; the properties to not have their descriptions; and the top-level object's description not to be generated at all.

Code to reproduce:

{
  "type": "object",
  "description": "Schema description",
  "properties": {
      "some property": {
          "type": "string",
          "description": "Description of the property"
      }
  }
}

This produces the following class for python 3.6 and 3.7:

class SchemaName:
    """Description of the property"""
    some_property: Optional[str] = None

What I would have expected:

class SchemaName:
    """Schema description"""
    # Description of the property
    some_property: Optional[str] = None

# alternatively:
class SchemaName:
    """Schema description

    Properties:
        some_property: Description of the property
    """"
    some_property: Optional[str] = None

seiltingcr avatar Mar 22 '21 13:03 seiltingcr

I upvote! :) a fix would make me very happy! :)

kopecn avatar Jul 26 '22 04:07 kopecn

https://docs.python.org/3.10/tutorial/controlflow.html#documentation-strings https://peps.python.org/pep-0287/

:)

kopecn avatar Sep 07 '23 20:09 kopecn

I took a look at the python renderer to see if this is a super easy fix, and it looks like the base class it uses is not quite flexible enough to allow this: It calls the same method (emitDescription) for both class descriptions and attribute descriptions, with no more data about what type of thing the description is for. It would be nice if someone who knows the codebase a little bit better could give a quick yes/no on the feasibility of this.

seiltingcr avatar Sep 15 '23 12:09 seiltingcr

There is also the fact that it produces quite antiquated-looking code. Even in this tiny example, it uses Optional[str], for instance, which I would not accept in a pull request in 2023. I am a bit wary of wasting time on a project that looks like 2 people have used it since March 2021.

seiltingcr avatar Sep 15 '23 12:09 seiltingcr

@seiltingcr I think this may have been fixed 2 months ago in a recent PR. --> https://github.com/glideapps/quicktype/pull/2443

I will be checking shortly.

kopecn avatar Jan 17 '24 23:01 kopecn