comic / grand-challenge.org

A platform for end-to-end development of machine learning solutions in biomedical imaging
https://grand-challenge.org
Apache License 2.0
177 stars 52 forks source link

Errors on updating (PUT) `ArchiveItem` and `DisplaySet` #3701

Open chrisvanrun opened 2 days ago

chrisvanrun commented 2 days ago

I was playing around with the rse-gcapi and exploring what is needed, and when for updating (PUT) a CIVSet (i.e. DisplaySet/ArchiveItem).

A few observations:

PUT ArchiveItem

{
  "values": [
    {
      "interface": "integer",
      "value": 13
    }
  ]
}

Requires an "archive" to be set, this is already an existing archive item, and should source from the current instance. Worse, this needs to be an api_url.

PUT DisplaySet

Same body as the above put, however:

AssertionError: The `.update()` method does not support writable nested fields by default.
Write an explicit `.update()` method for serializer `grandchallenge.reader_studies.serializers.DisplaySetPostSerializer`, or set `read_only=True` on nested serializer fields.
chrisvanrun commented 2 days ago

Uh, this needs some thought. Just going to use ArchiveItem here, but should apply both for ArchiveItems and DisplaySets.

Currently, we mainly seem to support patching (partial_update). I don't think PUT is being used by anyone (?). As for the API manipulations of items it's mainly about:

We don't readily have a way to delete values. Maybe that is where PUT could play a role. You define which CIVs you want, and the rest will automatically be removed. Albeit very few ppl will use this, I think a more common practical approach for users is to start from scratch remove all the items and re-start any upload script.

If we do decide to allow for PUT the following should be taken into account:

Updating (i.e. PUT) should remove all values and replace them with those provided, however, not sure if the order of execution is important. What if someone decides to re-use an image/file of a CIV that is already present? In that case, I think it best to first add the new values, check which have been overwritten, and then remove the remainder.

Partial updating (i.e. PATCH) should only add values but still support overwriting.