Skip to content

Future of JSON schema for Vega and Vega-Lite #86

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
domoritz opened this issue Mar 26, 2019 · 11 comments
Open

Future of JSON schema for Vega and Vega-Lite #86

domoritz opened this issue Mar 26, 2019 · 11 comments

Comments

@domoritz
Copy link
Member

domoritz commented Mar 26, 2019

This is a discussion. Please add your comments below.

I keep running into a wall when trying to use JSON schema with intersection types. I’d like to use anyOf but in combination with additionalProperties: false we end up with a schema that cannot be satisfied unless you only use properties that are defined in the intersection of the properties. So far, we have generated huge objects that merge properties but this is causing problems with intersections with unions (e.g. A & (B | C)). Going forward, I’d like to address this properly and see three options. 1) abandon JSON schema and use a more sophisticated schema enforcement 2) abandon additionalProperties: false, which means users won’t know if the mistype properties (this is what Vega does in some places), or 3) translate A & (B | C) to (A & B) | (A & C) and generate an anyOf of multiple large objects that are possibly largely the same. In the worse case, this can get exponentially larger. For example (A | B) & (C | D) = A & C | A & D | B & C | B & D Opinions?

See related issues: #62 and #4.

Another issue with JSON schema is inheritance, which cannot be expressed. So far we keep creating large objects for this case as well.

@jakevdp
Copy link

jakevdp commented Mar 26, 2019

If you don't use additionalProperties: false on the anyOf schema, but instead use it in each of the components of the anyOf, does that work?

Agreed on the lack of expressible inheritance: that's a real point of weakness.

@kanitw
Copy link
Member

kanitw commented Mar 26, 2019

The end of this page is probably relevant:

https://json-schema.org/draft-06/json-schema-release-notes.html

@domoritz
Copy link
Member Author

If you don't use additionalProperties: false on the anyOf schema, but instead use it in each of the components of the anyOf, does that work?

Actually, the problem is when you have additionalProperties: false on all of the components. Imagine one with {a: number, b: number} and one with {c: number, d: number}. Now, an object with c will violate the first and one with a will violate the second (each object individually does not allow additional properties).

@jakevdp
Copy link

jakevdp commented Mar 26, 2019

Maybe I'm not understanding... here's a concrete example where instances pass even with additionalProperties: false:

import jsonschema

schema = {
    'anyOf': [
        {'$ref': '#/definitions/A'},
        {'$ref': '#/definitions/B'}
    ],
    'definitions': {
        'A': {
            'properties': {
                'a': {'type': 'number'},
                'b': {'type': 'number'}
            },
            'additionalProperties': False
        },
        'B': {
            'properties': {
                'c': {'type': 'number'},
                'd': {'type': 'number'}
            },
            'additionalProperties': False
        }
    }
}

jsonschema.validate({'a': 1, 'b': 2}, schema)  # passes
jsonschema.validate({'c': 1, 'd': 2}, schema)  # passes

@domoritz
Copy link
Member Author

hmm, I tried the schema below in https://www.jsonschemavalidator.net/.

{
  "$ref": "#/definitions/BC",
  "$schema": "http://json-schema.org/draft-06/schema#",
  "definitions": {
    "B": {
      "additionalProperties": false,
      "properties": {
        "b": { "type": "number" },
        "x": { "type": "number" }
      },
      "type": "object"
    },
    "BC": {
      "allOf": [{"$ref": "#/definitions/B" },
                { "$ref": "#/definitions/C" }]
    },
    "C": {
      "additionalProperties": false,
      "properties": {
        "c": { "type": "number" },
        "x": { "type": "number" }
      },
      "type": "object"
    }
  }
}

Screen Shot 2019-03-25 at 23 15 30

@jakevdp
Copy link

jakevdp commented Mar 28, 2019

Difference is allOf vs anyOf

@domoritz
Copy link
Member Author

Ahh, I missed that. The thing is that intersection types need allOf.

@kayahr
Copy link
Contributor

kayahr commented Jun 5, 2019

I have to admit I'm not using Vega or Vega-Lite, I just use the schema generator. But i want to comment on it anyway: In my opinion it would be very sad if you abandon JSON schemas because I guess we would then loose an important driving force in the typescript-to-json-schema world. So out of selfish reasons let me use my Jedi powers and tell you this blunt lie: There IS no alternative to JSON schema out there, don't look for it! Instead keep up the good work on the schema generator, please! :-)

@domoritz
Copy link
Member Author

domoritz commented Jun 5, 2019

Thank you for the kind words @kayahr. I don't think you have anything to worry about.

Btw, I think the issue we are discussing here with intersection types will be resolved with draft 8: json-schema-org/json-schema-spec#656.

@rijenkii
Copy link

As of draft 2019-09 unevaluatedProperties was officially added.

Example schema for (A | B) & (C | D):

{
  "$ref": "#/$defs/root",
  "$defs": {
    "root": {
      "allOf": [
        {
          "oneOf": [
            { "$ref": "#/$defs/a" },
            { "$ref": "#/$defs/b" },
          ],
        },
        {
          "oneOf": [
            { "$ref": "#/$defs/c" },
            { "$ref": "#/$defs/d" },
          ]
        },
      ],
      "unevaluatedProperties": false,
    },
    "a": {
      "type": "object",
      "properties": { "a": { "type": "string" } },
      "required": ["a"],
    },
    "b": {
      "type": "object",
      "properties": { "b": { "type": "string" } },
      "required": ["b"],
    },
    "c": {
      "type": "object",
      "properties": { "c": { "type": "string" } },
      "required": ["c"],
    },
    "d": {
      "type": "object",
      "properties": { "d": { "type": "string" } },
      "required": ["d"],
    },
  }
}

You can test this schema on https://json-everything.net/json-schema/.
Official json-schema blog post mentioning unevaluatedProperties: https://json-schema.org/blog/posts/modelling-inheritance.

@domoritz
Copy link
Member Author

Oh cool. Thanks for the link. It would be nice to support that pattern for inheritance but it also looks tricky as we would need to enumerate all implementing classes. Definitely a bigger refactor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants