jsonschema import does not interpret defaults for objects

[tvandinther]

Bug Report

1. Minimal reproduce step (Required)

Given the following schema.json:

{
  "type": "object",
  "properties": {
    "foo": {
      "type": "object",
      "properties": {
        "bar": {
          "type": "string",
          "default": "baz"
        }
      },
      "required": ["bar"]
    }
  },
  "required": ["foo"]
}

kcl import schema.json --mode jsonschema generates:

"""
This file was generated by the KCL auto-gen tool. DO NOT EDIT.
Editing this file might prove futile when you re-run the KCL auto-gen generate command.
"""

schema Schema:
    r"""
    Schema

    Attributes
    ----------
    foo : SchemaFoo, required
    """

    foo: SchemaFoo

schema SchemaFoo:
    r"""
    SchemaFoo

    Attributes
    ----------
    bar : str, required, default is "baz"
    """

    bar: str = "baz"

And create a main.k of

Schema{}

2. What did you expect to see? (Required)

kcl run main.k schema.k produces

foo:
  bar: baz

and the contents of schema.k to be

"""
This file was generated by the KCL auto-gen tool. DO NOT EDIT.
Editing this file might prove futile when you re-run the KCL auto-gen generate command.
"""

schema Schema:
    r"""
    Schema

    Attributes
    ----------
    foo : SchemaFoo, required
    """

    foo: SchemaFoo = SchemaFoo{}

schema SchemaFoo:
    r"""
    SchemaFoo

    Attributes
    ----------
    bar : str, required, default is "baz"
    """

    bar: str = "baz"

Note the line

foo: SchemaFoo = SchemaFoo{}

where we instantiate the default version of the sub schema. If anything in the sub schema is required but has no default, the standard error will continue to throw. A suitable fix for me would be to make the default behaviour of the jsonschema import to create an instance of a schema as the default value for all schemas in a schema. Otherwise, it is not possible to use this import for creating nested structures with default values.

3. What did you see instead (Required)

kcl run main.k schema.k produces

attribute 'foo' of Schema is required and can't be None or Undefined

4. What is your KCL components version? (Required)

❯ kcl version
0.10.9-linux-amd64

[He1pa]

First of all, I don't think it's a good idea to give any schema a default value directly. This may lead to inconsistencies: some types have a default value (schema), and some don't (other base types)

Such as bar in this example，If we remove default value, but generate a default value for foo,

{
  "type": "object",
  "properties": {
    "foo": {
      "type": "object",
      "properties": {
        "bar": {
          "type": "string",
           # "default": "baz"  # romove this
        }
      },
      "required": ["bar"]
    }
  },
  "required": ["foo"]
}

like the following

"""
This file was generated by the KCL auto-gen tool. DO NOT EDIT.
Editing this file might prove futile when you re-run the KCL auto-gen generate command.
"""

schema Schema:
    r"""
    Schema

    Attributes
    ----------
    foo : SchemaFoo, required
    """

    foo: SchemaFoo = SchemaFoo{}

schema SchemaFoo:
    r"""
    SchemaFoo

    Attributes
    ----------
    bar : str, required, default is "baz"
    """

    bar: str

It still some error:
EvaluationError
--> /Users/zz/code/test/kcl/test1/schema.k:10:1
|
10 | foo: SchemaFoo = SchemaFoo{}
| attribute 'bar' of SchemaFoo is required and can't be None or Undefined

I think we need a more sound solution. Do you have any better suggestions?

[tvandinther]

I would argue your example is the correct behaviour since "foo" is a required property and therefore the sub schema should also be adhered to. If "foo" wasn't required, then the example gets a bit more difficult because then a choice needs to be made between instantiating the default schema or not. In which case I suppose it would make sense not to.

So if a property is required and is of type object, the resulting subschema should be instantiated.

Otherwise if the behaviour needs to be opt-in, some command-line flag (and SDK option) could be given to give this behaviour.

Alternatively giving SDK access to programtically building an AST and rendering that to file would also be immensely helpful.