Img data json parents

[will-moore]

As suggested by @chris-allan in #4446 (comment), this fixes the handling of multiple parents (Datasets & Wells) by marshalImage().

NB: I haven't changed handling of Projects yet.

@chris-allan suggested the "first" of multiple parents be used for keys such as "datasetId", although I'm not sure how we want to sort datasets before picking the first?

To test:

• Easy: put an image into multiple Datasets then inspect the json object at url: webgateway/imgData/3731/

You should see under the meta section a datasets list like this:

datasets: [
{
    description: "",
    id: 601,
    name: "CSFV"
},
{
    description: "Images in this Dataset are from ROIs of parent Image: Name: scram_01.r3d Image ID:   324",
    id: 3301,
    name: "From_ROIs"
}
],

and also datasetName, datasetId and datasetDescription based on the first dataset.

• Harder: Run the "Dataset_To_Plate" script without removing the image from the Dataset.
  Now check that you see multiple wells in the meta as above.

[atarkowska]

could you change to a if test else b, https://www.python.org/dev/peps/pep-0308/

[atarkowska]

@will-moore could you flake8 ./components/tools/OmeroWeb/test/integration/test_marshal.py:150:80: E501 line too long (85 > 79 characters)

[jburel]

@will-moore: we will need to indicate in documentation of imageMarshal how it works in the case of multiple parents

[jburel]

There is no test coverage for the wellsample/well case

[will-moore]

Follow up card for 5.3: https://trello.com/c/H1xBtuBh/21-imgdata-json-parents-cleanup

[chris-allan]

There is added problem of assumptions over who owns and the subsequent set of operations that are acceptable via the permissions system across all the parents. This is of particular interest for our use of parents when working with the metadata that this API presents.

I would not suggest us dealing with this here but rather consider keeping the scope of the changes in this PR narrow. We can then expand the API with a new fully featured, possibly using omero-marshal, set of image metadata available via JSON in the future.

[chris-allan]

Something that just came up in discussing the use of this is that we probably really need to add some context to the full viewer URL and view. This would be so that the viewer at least has the option of being reactive to the container that was open when the viewer itself was launched. Otherwise we are guessing.

Maybe parent=Dataset:<dataset_id>, parent=WellSample:<well_sample_id>, etc. added as a URL encoded parameter? Should be relatively easy to do with the current JavaScript that launches the viewer from double click or the "Full Viewer" button?

/cc @joshmoore, @knabar, @emilroz

[atarkowska]

Works as expected:

"meta": {
        "projectDescription": "",
        "datasetName": "atest1",
        "datasets": [
            {
                "description": "",
                "id": 6901,
                "name": "atest1"
            },
            {
                "description": "",
                "id": 6902,
                "name": "atest2"
            },
            {
                "description": "",
                "id": 6903,
                "name": "atest3"
            }
        ],
        "wellSampleId": 15901,
        "projectName": "Multiple",
        "imageDescription": "",
        "imageTimestamp": 1158540058,
        "wells": [
            {
                "wellSampleId": 15901,
                "id": 314251
            },
            {
                "wellSampleId": 15904,
                "id": 314252
            },
            {
                "wellSampleId": 15903,
                "id": 314252
            },
            {
                "wellSampleId": 15906,
                "id": 314253
            },
            {
                "wellSampleId": 15905,
                "id": 314253
            },
            {
                "wellSampleId": 15908,
                "id": 314254
            },
            {
                "wellSampleId": 15907,
                "id": 314254
            }
        ],
        "imageId": 8751,
        "imageAuthor": "user-1 user-1",
        "imageName": "IN-CSFV01_01_R3D_D3D.dv",
        "datasetDescription": "",
        "wellId": 314251,
        "projectId": null,
        "pixelsType": "uint16",
        "datasetId": 6901
    },

orphaned

"meta": {
        "projectDescription": "",
        "datasetName": "Multiple",
        "datasets": [],
        "wellSampleId": "",
        "projectName": "Multiple",
        "imageDescription": "JEFFERSON,THOMAS 12/31/9",
        "imageTimestamp": 1448011654,
        "wells": [],
        "imageId": 26501,
        "imageAuthor": "user-1 user-1",
        "imageName": "CT_APERT.hdr",
        "datasetDescription": "",
        "wellId": "",
        "projectId": null,
        "pixelsType": "uint8",
        "datasetId": null
    },

I couldsn;t put image in multiple plate as Script says Success Dataset To Plate Excluded 1 out of 1 dataset(s). I wrote own script

[atarkowska]

With the follow up trello card for 5.3 this is good to merge,

[chris-allan]

Issues of context available to the full viewer and integration test coverage should be resolved before merging.

[will-moore]

@chris-allan Maybe you could give more details of how the image viewer should handle different contexts? Currently the only context it knows about is for the Prev and Next buttons for moving through Dataset images or Search results. This is looked-up simply by using the parent opener window and using jQuery to find thumbnails matching the current image ID, then finding the prev/next in the DOM.
The viewer doesn't display any other context / parent data (Project, Dataset or Well) except using the Well to load bulk annotations.
Do you want to display the parents in an additional "postit" panel, indicating which of multiple parents is the current context (if any)? Would it not be better to add viewer functionality in a different PR from the imgData change that is simply to fix the loading of bulk annotations (from #4446).

I'll add Well context to the integration tests.

[chris-allan]

I'm not suggesting that we change what the full image viewer does currently or that we add anything to it in this PR. All that a full image viewer has to go on at this point is the information it receives via the URL, its parameters, and the imgData JSON. Before this PR, all full image viewers had to assume based on the metadata provided from these sources that there was only one parent. This is now, and quite correctly, no longer the case. Any development on a full viewer going forward with respect to parent display or parent related user interface concepts would need to both be aware of multiple parents and be informed of the parent that was open when the full viewer was launched.

What I am proposing is that we establish a defined and documented entry point, via the opening URL, by which the full image viewer could then do something with the parent which has been provided in an authoritative manner. This is especially important with #4480 merged in and in use both for proof of concept work and plugins in combination with this PR. It is also of low cost and immense benefit to establish. We have a very small number of locations where the full image viewer is started via OME.openPopup(). All of these locations should be more than capable of establishing the context in which they are invoked and passing it along.

Interrogating the parent window's DOM to try and find this out is not an extension point that I would suggest anyone use. Certainly not in any sort of backwards or forwards compatible way.

[will-moore]

@chris-allan Agreed that all you suggest should be done, but not for 5.2.2 since we are past "feature freeze". Even though the javascript changes might be relatively trivial, it is still a bit of new code to write, add tests for, manually test etc.
Querying the parent window's DOM was originally an external PR that we accepted since it performed useful function and had been requested by a number of users, but not the ideal way of doing it (very fragile, broke in 5.2.0, fixed in #4485 with robot tests now!)

[chris-allan]

All accepted.

We should just be mindful that opening the full viewer up to multiple parents is a pretty significant can of worms. I'm almost of the opinion that we leave this particular PR out for now until we can get a handle both from a workflow and development perspective on the impact. Tough call.

[will-moore]

I'm happy to leave this PR out of 5.2.2.
However, adding this extra info to imgData json doesn't mean that the image view must use this. It already ignores most of the existing parent info (as discussed above). This PR simply fixes display of bulk annotations (on Screen/Plate) when the image is also in Dataset, which is a very edge-case bug so not essential for 5.2.2.

[chris-allan]

The issue I am trying to raise is that the change is not just scoped to fixing display of bulk annotations when the image is also in a Dataset within the current full viewer. imgData JSON is public API. To give you a concrete example of this, we use the existing parent information both in the DataViewer version of the full image viewer and in the PathViewer to do a variety of context specific things such as showing thumbnails or additional metadata.

You can see an example of this in the JCB DataViewer full image viewer here:

• http://jcb-dataviewer.rupress.org/jcb/img_detail/634060/28658/

This PR changes what each would display in situations of multiple parents and I'm cool with that in principal. What I'm concerned about are the implications on users of doing so and what corners we back ourselves into when we have a lack of context available to full image viewer implementations. We can certainly argue the utility and correctness of not showing anything vs. the possibility if showing something incorrect or unexpected separately.

I would really like to avoid instituting hacks in our own APIs that could be avoided if we do things properly and thoroughly from the start.

[jburel]

Marking this PR onhold since it will not be considered for 5.2.2
We need to carefully plan the changes, we could use the "design" repo (issues) or card.
Let's discuss post release how to move forward.

[will-moore]

@jburel @chris-allan Are we ready to continue / restart the discussion on this PR?
To recap, we discussed fixing the webclient to open images with webclient/img_detail/<imageId>/?dataset=<datasetId> so that the image viewer could know it's context.
If this is going into 5.2.3 then we wouldn't want to remove the datasetName, datasetId attributes from the imgData (breaking change) but if we're holding off for 5.3.0 then these could be removed along with similar Project attributes.
Another change that could be added to this PR is to list "projects": [{"id": ...etc under each dataset.

[jburel]

As mentioned, I think we should move the discussion to design.

[will-moore]

After discussing in web meeting today, decision is that we will not add or change what we have now in imgData json. We can already query paths_to_object as we do for jsTree traversal to image to get Projects/Datasets parents.
Focus instead on using image marshal and new web api to provide 'right' way to do this.