search

jsongraph / json-graph-specification

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

Changes to BEL JGF child schema #16

Closed abargnesi closed 9 years ago

abargnesi commented 9 years ago

Warning

:coffee: or :tea: recommended

Motivations

There are number of deficiencies within the BEL child schema where JSON properties should have schema or schemas need to be reorganized. The known issues are:

  1. No standard property for capturing language version for OpenBEL.
  2. Missing evidence.metadata. We need a JSON object to capture data about the evidence object that is not part of a biological experiment (e.g. _evidence.experimentcontext).
  3. Change the _evidence.biologicalcontext property to _evidence.experimentcontext. The rationale for this is described in OpenBEL/openbel-server#51.

    Breaking changes

The following are all potentially breaking changes since we are applying new schema to properties that user's may have specified due to additionalProperties.

(1)

Introduce a _graph.metadata.belversion property ("string" type) to capture the version of BEL used in the JGF. It will be documented that the value is in conformance with BEL version guidelines, but I would rather not constrain the value any further. This property is optional and we should RECOMMEND implementers default to the latest version of BEL.

(2)

Rename the _evidence.biologicalcontext property to _evidence.experimentcontext. An experiment_context captures the intended use. @ncatlett describes the motivations for the change in OpenBEL/openbel-server#51:

Can we rename biological_context to experiment_context? My feeling is that the context annotations should reflect the experiment information (which may not be "biological") and the metadata should reflect information about the curation.

(3)

Introduce an evidence.metadata property ("object" type) to capture data about the evidence object. This could be useful for the following uses:

Answer: Setting additionalProperties to false for evidence and evidence.citation matches the intended constraints of the schema. In the future this will reduce the likelihood of breaking changes between the schema and properties introduced in the data instance.

nbargnesi commented 9 years ago

I'd like to see additionalProperties stay false. To minimize breaking changes, it would be worse if we allow properties we don't control. If additional properties are being overlaid on the BEL JGF child schema, the current schema isn't doing its job and that needs to be addressed.

Allowing for the additional properties in metadata like we currently do seems reasonable though.

If an application needs additional properties outside metadata, let them be defined as siblings or parents to the schema-defined data. If the BEL JGF specification doesn't identify what BEL JGF data looks like precisely, it's not a specification.

abargnesi commented 9 years ago

I agree with your view on additionalProperties. Currently we do not specify additionalProperties for evidence or evidence.citation. In this case I believe the default is either true or an empty schema (i.e. {}). Both of these cases will allow additional properties.

The JSON Schema Validation (http://json-schema.org/latest/json-schema-validation.html#anchor64) document isn't clear on this though.

adifabio commented 9 years ago

Heya,

I have no problem with any of the changes mentioned; versioning and metadata on evidence for experimentation are 2 real world issues I encountered recently using the specification and they do need to be addressed. I also agree with Nick in that additionalProperties should stay false.

Now that we have a bit of discussion bubbling around JGF and it's BEL child schema, I would like to see some form of Provence added to the main JGF specification allowing a network to intelligently define its ancestry. I also vacillate on if there should be an optional source property on the root as well. Being able to see quickly where the file was generated is useful.

Anselmo

On Thu, Jul 2, 2015 at 6:42 PM Anthony Bargnesi notifications@github.com wrote:

I agree with your view on additionalProperties. Currently we do not specify additionalProperties for evidence or evidence.citation. In this case I believe the default is either true or an empty schema (i.e. {}). Both of these cases will allow additional properties.

The validation spec isn't clear on this though - http://json-schema.org/latest/json-schema-validation.html#anchor64 On Jul 2, 2015 6:28 PM, "Nick Bargnesi" notifications@github.com wrote:

I'd like to see additionalProperties stay false. To minimize breaking changes, it would be worse if we allow properties we don't control. If additional properties are being overlaid on the BEL JGF child schema, the current schema isn't doing its job and that needs to be addressed.

Allowing for the additional properties in metadata like we currently do < https://github.com/jsongraph/json-graph-specification/blob/master/child-schemas/bel-json-graph.schema.json#L77

seems reasonable though.

If an application needs additional properties outside metadata, let them be defined as siblings or parents to the schema-defined data. If the BEL JGF specification doesn't identify what BEL JGF data looks like precisely, it's not a specification.

— Reply to this email directly or view it on GitHub < https://github.com/jsongraph/json-graph-specification/issues/16#issuecomment-118184673

.

— Reply to this email directly or view it on GitHub https://github.com/jsongraph/json-graph-specification/issues/16#issuecomment-118186405 .

abargnesi commented 9 years ago

@adifabio An ancestor chain would be useful for some applications. Please put together a proposal under a new issue.