Fix for #93

[Ownmarc]

Quick fix for #93 without diving in react-image-annotate

[Ownmarc]

Works well for Segmentation, but I am getting bugs with classification (and I didn't test the others).

[Ownmarc]

Works well for Segmentation, but I am getting bugs with classification (and I didn't test the others).

fixed by checking if we are in image_segmentation

[seveibar]

I'm not sure if this is the right level of abstraction to fix #93 because some people may cycle through their image annotation, and the "empty" state is actually the "done" state for purposes of displaying the samples (example below helps clarify this). I've asked a question on #93 which will help clarify the level of abstraction that is appropriate, but I have a feeling the right place for this functionality is in the <ImageSegmentation /> component or the useLocalStorage file-handler

Consider the following object created in the current version of the editor with the image segmentation template. Notice that the last sample has an empty array rather than null, because I hit save with no regions selected.

{
  "interface": {
    "type": "image_segmentation",
    "availableLabels": [
      "valid",
      "invalid"
    ],
    "regionTypesAllowed": [
      "bounding-box",
      "polygon",
      "point"
    ]
  },
  "taskData": [
    {
      "imageUrl": "https://s3.amazonaws.com/asset.workaround.online/example-jobs/sticky-notes/image1.jpg"
    },
    {
      "imageUrl": "https://s3.amazonaws.com/asset.workaround.online/example-jobs/sticky-notes/image2.jpg"
    }
  ],
  "taskOutput": [
    [
      {
        "regionType": "bounding-box",
        "centerX": 0.2962962962962963,
        "centerY": 0.3313253012048193,
        "width": 0.3891120035698349,
        "height": 0.15127175368139223,
        "color": "hsl(139,100%,50%)"
      }
    ],
    []
  ]
}

After this change (if I understand the change correctly), upon repeating the steps I did above this object would become...

{
  "interface": {
    "type": "image_segmentation",
    "availableLabels": [
      "valid",
      "invalid"
    ],
    "regionTypesAllowed": [
      "bounding-box",
      "polygon",
      "point"
    ]
  },
  "taskData": [
    {
      "imageUrl": "https://s3.amazonaws.com/asset.workaround.online/example-jobs/sticky-notes/image1.jpg"
    },
    {
      "imageUrl": "https://s3.amazonaws.com/asset.workaround.online/example-jobs/sticky-notes/image2.jpg"
    }
  ],
  "taskOutput": [
    [
      {
        "regionType": "bounding-box",
        "centerX": 0.2962962962962963,
        "centerY": 0.3313253012048193,
        "width": 0.3891120035698349,
        "height": 0.15127175368139223,
        "color": "hsl(139,100%,50%)"
      }
    ],
    null
  ]
}

I think this new behavior is probably OK. I just want to mention there's a use case where someone wants to mark a sample as "done" after reviewing it, and an empty array is also an OK way to say, "there are no regions". If the concern of #93 is performance, we should considering modifying useLocalStorage with a periodic check, e.g. every 10 seconds if it has changed. If that creates stuttering, we could also modify how objects are stored in local storage to make writes smaller.

This PR can be considered a checkpoint and merged, just wanted to document some thoughts. See the small changes I mentioned in the code.

[seveibar]

something very useful about seamless immutable objects is you can usually just do o1 === o2

[Ownmarc]

that compares keys length ?

[Ownmarc]

You understand it correctly. [] would be the way to say there is nothing in it and null would be a state where no one has passed on the image yet. Maybe there could be something added in the annotation to explicitly say what state the annotation is in (example: has to be reviewed, reviewed, final, ground truth, etc.)

null ->[] would trigger onChangeOHA whil [] -> [] would not trigger onChangeOHA

[seveibar]

So the fix here is very related to #32 and sample state being unclear. @Ownmarc is right that we need to represent the state of sample better. I think there's a good potential to do this while integrating the brush proposal into the UDT JSON. We're pushing out some awesome changes this week to make UDT work as a importable React or native javascript library (the last npm module is pretty out of date) but then I think this issue will take full focus. I'll make sure to CC everyone here on the format change proposal and include sample state management.

@Ownmarc curious if you have any thoughts on #32, I'm not sure if we should introduce JSONL yet. I do think this format would be more intuitive and make it clearer how to represent sample state:

{
  "interface": { /* ... */ },
  "samples": Array<{
      /* document, imageUrl etc. */
     "state": "needs-review",
     "output": {
      /* entities etc. */
     }
   }>
}