jsongraph / json-graph-specification

A proposal for representing graph structure (nodes / edges) in JSON.
http://jsongraphformat.info/
Other
446 stars 36 forks source link

Spec Modernization - Phase 1 #43

Closed tgig closed 4 years ago

tgig commented 4 years ago

Hi all, this is a significant update, I'll do my best to describe everything and start a discussion.

Major topics for discussion:

  1. Update to JSON Schema Draft7
  2. Remove json-schema b/c it is no longer supported, replace with json_schemer b/c it works for Draft7
  3. Add $id and title fields to the schema definition, best practices for Draft7
  4. Changed the nodes array into keyed objects, as discussed in #39. (Please see important further detail about this change below)
  5. Add id field back into the graph object
  6. Simplify the graphs array, removing some of the unused properties. Remove null types from properties objects, as this is built-in functionality
  7. Phase 2?? What's next (see comments below)

1. Update to JSON Schema Draft7

There have been some best practices and significant updates since Draft4 came out, so I thought it would be a good idea to update to Draft7. This is my first time implementing a JSON Schema, so if you see anything awry, let me know.

If you'd prefer to keep Draft4 in place, let's start a discussion about that.

2. Remove json-schema. Add json_schemer

json-schema is no longer supported, and does not work for Draft7. The only other ruby validator that works for Draft7 is json_schemer. I am not super happy with this one, it doesn't have the same functionality, and error code opacity is a problem. It does not support schema validation, so I had to add the Draft7 schema as a file in order to validate that our json-graph-specification schema is a valid format.

3. Add $id and title fields

This seems to be best practice for every Draft7 schema that I used as reference. So, adding here as well. We should update the $id uri location to somewhere that actually hosts the file - a github.io address, or publish the final json schema to jsongraphformat.info. Not the end of the world if our schema does not exist at the uri, but it is best practice.

I think this should close #27

4. Change nodes array to objects

We discussed this mod briefly in #39 and it seemed logical to me. I removed the array and made it an object, removed the id property and using it as a key (see examples).

In my estimation, it does not make sense to also turn the edges array into keyed objects, because that would require a unique id for each edge. As of now the id property is not required, and by turning it into objects, it would be required. This StackOverflow convo is a pretty good example of the challenge somebody else already faced in this regard.

If approved, should close #39

5. Add id field back to graph object

Should close #32

6. Simplify

Some general housekeeping, cleaning up. See diff and let me know if you have any questions or concerns about the mods, I think they're pretty straightforward.

7. Phase 2??

It seems to me there is still some low hanging fruit here.

wshayes commented 4 years ago

Great work!

I like your suggestions:

I can convert the docs in a separate PR once this one is merged.

I'll also add the schema to the website as you indicated in the $id field.

wshayes commented 4 years ago

The schema for nodes - allows anything in that object to be added:

        "nodes": {
          "type": "object",
          "properties": {
            "label": {
              "type": "string"
            },
            "metadata": {
              "type": [
                "object"
              ]
            }
          }
        },

Can you structure it with a stricter definition like: https://stackoverflow.com/questions/27357861/dictionary-like-json-schema/27375654

wshayes commented 4 years ago

We should label this as version 2 in the $id and tag the current version at V1.

tgig commented 4 years ago

Thanks @wshayes

Can you structure it with a stricter definition

Done. Check out the latest commit and let me know if this is satisfactory.

We should label this as version 2 in the $id and tag the current version at V1.

Done. I added a /v2.0/ folder, which will allow us to keep the same file name, but add new versions over time.

Let's go ahead and convert over to python. I have a lot more experience with python than ruby. I've been using markdown over restructuredtext more and more so happy to support that as well.

Will wait for the next pull request for these items.