cytoscape / cytoscape.js

Graph theory (network) library for visualisation and analysis
https://js.cytoscape.org
MIT License
10.09k stars 1.64k forks source link

documentation for elements JSON serialisation is unclear #2201

Closed kortschak closed 5 years ago

kortschak commented 6 years ago

documentation

Bug report

Environment info

Current (buggy) behaviour

The current documentation describing the structure of the elements JSON shows that all graph elements are placed in a single array that then has the individual array elements optionally differentiated by a "group" property.

  elements: [
    { // node n1
      group: 'nodes', // 'nodes' for a node, 'edges' for an edge
      // NB the group field can be automatically inferred for you but specifying it
      // gives you nice debug messages if you mis-init elements

      data: { // element data (put json serialisable dev data here)
        id: 'n1', // mandatory (string or number) id for each element, assigned automatically on undefined
        parent: 'nparent', // indicates the compound node parent id; not defined => no parent
      },

<snip>

However, documents in the wild and used as examples show that the "elements" property is an object {"nodes": [], "edges": []}.

Code in the documentation js appears to support the form shown in the wild over the documentation above.

Maybe it's the case that both approaches are available. This and this suggest that is true.

Desired behaviour Clarity. If both forms are acceptable both should be described, and it seems from looking at the json in the wild the form not shown is more common (and I'd argue clearer and more efficient).

Minimum steps to reproduce Read documentation.

maxkfranz commented 6 years ago

The plain array format is the preferred form. Eventually the { nodes: [], edges: [] } form will be deprecated with a warning in the console, and then it will be removed in v4. For now, either works. The { nodes: [], edges: [] } form is a remnant from Cytoscape Web -- the predecessor of Cytoscape.js.

There are inconsistencies in the examples and demos regarding this due to inertia. I'm making an effort with the 3.3 release to do a full pass of the docs and demos to clean things up. I'll keep this formatting issue in mind.

kortschak commented 6 years ago

Thanks.

The reason this came up is https://github.com/gonum/gonum/pull/621 where I add Cytoscape.js serialisation support in https://github.com/gonum/gonum/pull/621/commits/76d94d72aedea5aae83d13f387992d67308c72ed. Given that the deprecated form exists, and code will continue to exist in the wild, I'll keep both forms, but can note that the object form is deprecated when that happens, sadly from a perspective of efficiency (graph size and order are constant time operations in that form, but are O(n+m) in the plain array format).

maxkfranz commented 5 years ago

On further thought: I don't think it will be practical to drop support for they keyed { nodes: [], edges: [] } format for some future v4 release.

However, I've added docs in v3.3 to cover the two options in the Notation > Elements JSON section. There is also a flat option to cy.json(). If you call cy.json(true), then you will get the preferred flat array format. In v4, there will be a breaking change such that flat is true by default -- whereas it is false by default now.

kortschak commented 5 years ago

Thank you. The code we have will be unaffected by that as it's the responsibility of the user to use the appropriate import type in our package.

I'm happy for this to be closed unless you want it kept open for the breaking change at v4.

maxkfranz commented 5 years ago

I'll create a separate ticket for that when we're closer to v4.

Closing