lnoor / sphinx-jsonschema

A Sphinx extension to display a JSON Schema
http://sphinx-jsonschema.readthedocs.io
GNU General Public License v3.0
73 stars 23 forks source link

Table doesn't state which attributes are required or not #78

Open GlenNicholls opened 2 years ago

GlenNicholls commented 2 years ago

After the 1.19.0 release, I attempted to document my python object. Anyways, my schema looks sort of like this:

schema: dict = {
    "$schema": "https://json-schema.org/draft/2020-12/schema",
    "title": "schema",
    "type": "object",
    "required": ["core_spec", "cores"],
    "properties": {
        "core_spec": {
            "type": "number",
            "enum": [1, 2],
            "description": "Core specification that is used to parse the core file",
        },
        "description": {
            "type": "string",
            "description": "The general description of the core file",
        },
        "cores": {
            "type": "array",
            "items": {"$ref": "#/$defs/core"},
            "minItems": 1,
            "uniqueItems": True,
            "description": "Array of cores the core file defines",
        },
    }
}

I'm intentionally leaving out the defs as I don't think they're relevant and they're also not something I can share. As you'll see above, core_spec and cores should both be required but description should be optional. In the produced documentation, I'm not seeing any indication of this:

image

I think there should be an extra field stating if the object is required or not.

Anyways, I'm super stoked that the feature in 1.19.0 works, this looks much better than I was hoping and is certainly much more elegant than the table I was maintaining manually 👍

lnoor commented 2 years ago

Required fields are indicated by bold type. It is difficult to see in your screen dump but it appears that both core_spec and cores are bold and description is not. Please reopen if this is not correct.

GlenNicholls commented 2 years ago

I see that now, at least with the read the docs style above, the contrast difference is not very obvious.

What are your thoughts on changing this to be explicit with a new line item for each property stating if it is required or not? I am using schema's to define configuration files so the less I have to document outside the table for the user the better.

lnoor commented 2 years ago

Let's start by saying that I haven't found this to be a problem. What mattered to me that this solution worked well for html, pdf and epub documents. That doesn't mean that it is the only 'allowed' solution. I do care that this notation remains possible.

Alternatives: anything you can think of. You can experiment by manually coding an rst table and try out what works for you. In the end that is all that the sphinx-jsonschema does: convert a json schema into a rst table.

So you could replace the bold print with class then use CSS to turn it in whatever you want. Or include a superscripted req after the property name.

What worries me a bit is that the number of options and switches is expanding really fast. I was thinking: how about making a required_format() function replacing the code in wide_format.py lines 328-331? Default action would be equal to the current implementation but this would let you create an alternative in conf.py to replace the default. I assume that within a documentation project you would always use the same method to indicate if a property is required.

Would that work for you?

GlenNicholls commented 2 years ago

What worries me a bit is that the number of options and switches is expanding really fast.

I see what you mean and I agree. For my projects, there's a limited subset of JSON schema that I use, and have a preference for how I think it should look. With that said, I do understand that others have different preferences.

I was thinking: how about making a required_format() function replacing the code in wide_format.py lines 328-331? Default action would be equal to the current implementation but this would let you create an alternative in conf.py to replace the default. I assume that within a documentation project you would always use the same method to indicate if a property is required.

Would that work for you?

It would work for me, however, I don't think this is critical to change considering the bold-face formatting already identifies required properties. The feedback in my previous comment was to point out that it wasn't obvious to me. I can live with the current implementation, but you're welcome to leave this open if you see value in providing this capability or want feedback from others.

bollwyvl commented 3 weeks ago

Another approach: any pure string-to-string conversion (not the opaque cell data model) could be refactored to a jinja2 template on disk, with well-known file names which can be overloaded by lookup path.

In this specific case:

# conf.py
jsonschema_options = {
  "template_path": ["./_templates/jsonschema/"],
  "template_context": {}  # add extra filters, etc.
}

In the code, the directive would stand up a jinja2.Environment which would pick custom files first, falling back to the originals.

def __init__(self):
   # ...
    self.env = jinja2.Environment(
        loader=jinja2.FileSystemLoader([*self.options.template_path, SHIPPED_TEMPLATES])
    )

def _objectproperties(self, schema, key):
    # ...
    label = self.env.get_template("property_label.j2.rst").render(prop=prop, schema=schema, self.template_context)

The as-shipped property_label.j2.rst could look something like:

- {% if prop in schema.get("required", []) -%}**{{ prop }}**{%- else -%}{{ prop }}{% endif %}

... and could be overloaded by creating a single file, e.g. to remove the bullet, and add a superscript:

{% if prop in schema.get("required", []) -%}**{{ prop }}** :sup:`*`{%- else -%}{{ prop }}{% endif %}
lnoor commented 2 weeks ago

That is indeed a different approach! I suppose the template should render RestructuredText? I like the idea. However I am suffering from a burnout and am not actively working on the repositories. I'd consider and welcome help. Would you be willing to create a template based solution in addition to the wide_format formatter, a template_format?