cytoscape / cyREST

Core App: REST API module for Cytoscape
https://github.com/cytoscape/cyREST/wiki
MIT License
30 stars 13 forks source link

VisualMapping data is inconsistent between input and output #40

Closed dotasek closed 6 years ago

dotasek commented 6 years ago

For example, GET /v1/styles/{name} returns:

 {
      "mappingType": "continuous",
      "mappingColumn": "degree.layout",
      "mappingColumnType": "Number",
      "visualProperty": "NODE_SIZE",
      "points": [
        {
          "value": 1,
          "lesser": "1.0",
          "equal": "40.0",
          "greater": "40.0"
        },
        {
          "value": 18,
          "lesser": "150.0",
          "equal": "150.0",
          "greater": "1.0"
        }
      ]
    }

But POST /v1/styles will fail with the same input included, requiring this change:

{
      "mappingType": "continuous",
      "mappingColumn": "degree.layout",
      "mappingColumnType": "Integer",
      "visualProperty": "NODE_SIZE",
      "points": [
        {
          "value": 1,
          "lesser": "1.0",
          "equal": "40.0",
          "greater": "40.0"
        },
        {
          "value": 18,
          "lesser": "150.0",
          "equal": "150.0",
          "greater": "1.0"
        }
      ]
    }
dotasek commented 6 years ago

From a conversion with @bdemchak:

The major difference is "mappingColumnType", which is "Number" in the output, and requires a Cytoscape compatible type like "Integer" in the return. So what is output won't be accepted back.

There may be a simple solution; within CyREST, when given a column name, we can get "mappingColumnType" directly from Cytoscape and ignore it in input JSON. That way, we can support the same input, and maintain API.

dotasek commented 6 years ago

Turns out, the issue was that the same Java method was being used for both Columns and Visual Style Mappings, when in fact they both need to treated differently.

Columns cannot handle the "Number" type, so must be a primitive (Double, Integer, etc), and as such we treat "Number" as "Double" in this instance.

Mappings can use the actual Number class, so we can simply use "Number" on the input.

Fixed in 1c910e1

Awaiting release

dotasek commented 6 years ago

Closed in 3.6.1 release