PlateAcquisition filtering with plate display

[Tom-TBT]

Follow up on https://forum.image.sc/t/omero-web-plate-display-with-multiple-plateacquisition/90071

Rather than switching the plate display using the JSON api (my understanding is that it would require rewritting quite of JS to populate the plate with the results), I figured that I was able to pass the plateAcquisition ID (already exists and in use to filter the content of the "bird eye" in plate.html).

Passing the plateAcquisition ID down to PlateGrid, it adds a filter on which WellSamples are loaded. plate.html does the rest by populating only the Wells that contains WellSamples. Further selection of a well will show in the bird eye only the WellSample of that acquisition (as it already did).

If no plateAcquisition is passed (that's the case when the plate is selected in the tree), then all wellsample are loaded (as it did before). The difference in this case is that there is no filter on the "bird eye" when selecting a well, so all wellsamples are shown (which is the expected behaviour).

For it to show the plate when there is more than one run, I just removed the logic that prevented the plate to be loaded with more than one run (which was not preventing extra loading, since everything was loaded when selecting a plateacquisition)

The field index also seems to work as expected, without additional change.

[will-moore]

Thanks @Tom-TBT
Checks are failing with flake8 omeroweb/webgateway/views.py:1657:23: F821 undefined name 'long'

[Tom-TBT]

Hi Will,
I am a bit offended by flake8 because literally four lines above there is another call to long 💀
And it seems rather an issue with flake8, as long is defined with the imports:

omero-web/omeroweb/webgateway/views.py

Lines 64 to 67 in ab843f1

	try:
	import long
	except ImportError:
	long = int

Maybe something with the try except that flake8 isn't handling well with imports.

Anyway, I read about long to figure out why it's in use here, and it just seems Python2-related.
Since Python 2 isn't supported anymore, should I get rid of long completely?

[will-moore]

Yes, just use int instead.

[sbesson]

NB: this import block has been removed in https://github.com/ome/omero-web/pull/531/files where we cleaned up a lot of the legacy Python2/3 compatibility code. If you merge origin/master into your branch (which is what the CI build is doing under the hood), you will be able to reproduce the issue locally

[Tom-TBT]

The field index also seems to work as expected, without additional change.

I was wrong. There is still the issue when there's a different number of WellSample per Run between Wells. Note that the issue existed before these changes.

Discussing it locally, we figured that there are several use cases where this occurs (we have such datasets):

• Acquisition of subsets of Wells in a Run (not all wells are acquired in each Run)
• Automatic detection of objects and acquisition (each well will have different number of sample within a Run)

Here is the test data for the problem I described bellow
Test_uneven_wells.zip

Four wells, and a first Run with 3 sample in C3 & D4, and 1 sample in C4 & D3.
A second Run with 1 sample for each

Selecting the second Run, there are three fields listed (when there should be only one). First field shows C4 & D3, second shows nothing, third shows C3 & D4:

This is because the Field# is matched to the WellSample index (indexing done per Well):

Well | ws index | Acquisition 
-----------------------------
 C3  |     1    |     1
 C3  |     2    |     1
 C3  |     3    |     1
 C3  |     4    |     2
 C4  |     1    |     1
 C4  |     2    |     2
 D3  |     1    |     1
 D3  |     2    |     2
 D4  |     1    |     1
 D4  |     2    |     1
 D4  |     3    |     1
 D4  |     4    |     2

Selecting Acquisition 2, the index is in the range[2,4], given by plate.getNumberOfFields(run_id), and mapped to a field index this way:

omero-web/omeroweb/webclient/forms.py

Lines 384 to 388 in 4d8d25e

	class WellIndexForm(forms.Form):
	def __init__(self, *args, **kwargs):
	super(WellIndexForm, self).__init__(*args, **kwargs)
	rmin, rmax = kwargs["initial"]["range"]
	choices = [(str(i), "Field#%i" % (i - rmin + 1)) for i in range(rmin, rmax + 1)]

So in the case of the second run:

Field#1 -> index 2 (C4 & D3)
Field#2 -> index 3 (no such index at Acquisition 2)
Field#3 -> index 4 (C3 & D4)

where indexes are used by this query:

omero-web/omeroweb/webgateway/plategrid.py

Lines 49 to 61 in 4d8d25e

	query = (
	"select well.row, well.column, img.id, img.name, "
	"author.firstName||' '||author.lastName, "
	"well.id, img.acquisitionDate, "
	"img.details.creationEvent.time, "
	"img.description "
	"from Well well "
	"join well.wellSamples ws "
	"join ws.image img "
	"join img.details.owner author "
	"where well.plate.id = :id "
	"and index(ws) = :wsidx"
	)

I am investigating how I can solve this issue. Now trying with HQL queries to get the index adjusted to the Run:

SELECT 
    w.id,
    index(ws) - (
        SELECT MIN(index(ws_inner)) 
        FROM Well w_inner 
        JOIN w_inner.wellSamples ws_inner 
        WHERE ws_inner.plateAcquisition.id = :runid
        AND w_inner.id = w.id
    ) AS adjusted_index
FROM Well w 
JOIN w.wellSamples ws 
WHERE ws.plateAcquisition.id = :runid

[will-moore]

So, @Tom-TBT you want the number of Fields for any Acquisition to be the max(field_count) for that acquisition.
In your example, the field_count for all Wells for Acquisition2 is 1, so there should only be a single Field in the UI?

As was mentioned yesterday, some tests are failing at https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-test-integration/382/#showFailuresLink

They are mostly failing with:

TypeError: __init__() missing 1 required positional argument: 'acqid'

with also one of:

>           if self.acquisition > 0:
E           TypeError: '>' not supported between instances of 'str' and 'int'

[will-moore]

Tom, in the screenshot above where you've annotated the screenshot in Red and Green Run1 and Run2, I think it would be really useful if the UI could do this for you. When you're loading well children without filtering by Acquisition E.g. /webgateway/well/19106/children/ you'd need to add the Acquisition info to each Image returned, e.g.

    "acquisition": {"id": 123, "name": "01 - Uneven Run"}

Obviously the UI changes would need a bit of thought about the best way to show the different Acquisitions that the Fields belong to, so it doesn't need to happen in this PR (although could maybe add it as a tooltip in this PR to help debugging)?

[Tom-TBT]

@will-moore

you want the number of Fields for any Acquisition to be the max(field_count) for that acquisition.

Yes exactly. I implemented that in my last commit in webclient/views.py
I now pass to WellIndexForm the field range in [0, max(field_count)], and I modified the query in PlateGrid to return the WellSamples at these "offset indexes".

the field_count for all Wells for Acquisition2 is 1, so there should only be a single Field in the UI

Yes, that's what we should see now.

And with the fix on PlateGrid.__init__, the tests shouldn't fail.

@sbesson
I changed PlateGrid init for backward compatibility. I expect the behavior to be identical to what it was before my changes.
I added doc about acqid to explain that fid should be 0-indexed for the given acquisition (when using acquisition).

[Tom-TBT]

Some notes (for myself at least) about the UI changes you proposed Will:

Can add Run information in here.

omero-web/omeroweb/webgateway/views.py

Lines 1833 to 1839 in 99aebd2

	if img is not None:
	m = img.simpleMarshal(xtra=xtra)
	pos = marshal_pos(ws)
	if len(pos.keys()) > 0:
	m["position"] = pos
	wellImgs.append(m)
	return wellImgs

Something like m["name"] += f" Run:{run_name}" would append to the tooltip the name of the run, for the HTML generated here:

omero-web/omeroweb/webclient/templates/webclient/data/plate.html

Lines 270 to 273 in 99aebd2

	html += '<li ' + cls + ' title="' + w.name.escapeHTML() + '" data-imageId="' + w.id + '">';
	html += '<a href="{% url 'webindex' %}img_detail/' + w.id + '/">';
	html += '<div class="wellSize"><img /></div>';
	html += '</a></li>';

[will-moore]

This looks good now, using the sample file above:

• Viewing the second run shows just a single Field
• WellSamples are filtered correctly in Run 1 and Run 2 - all are shown when the Plate itself is selected in the tree
• Tooltips on the WellSamples showing which Run they come from

[will-moore]

Great that this is documented here, but I think more extensive explanation could be useful, since I'm not sure I would understand the behaviour here if I hadn't also read the code.
Using acqid changes the behaviour of the field index so that it's not the absolute field index but the relative field index for the fields of each Well after filtering by acquisition.

[will-moore]

Minor suggestion: Could you maybe just omit acquisition from the URL if it's not defined? Using 0 instead of an ID is a little confusing.
And the same is true elsewhere where acquisition should be None if not used instead of 0: e.g. The default (null) argument above acqid=0 feels like it should be acqid=None to indicate no filtering?

[will-moore]

Looks good, thanks 👍

[knabar]

Code looks good to me

[imagesc-bot]

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/how-does-omero-assign-wellsample-to-fields/100110/3

[imagesc-bot]

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/creating-multiple-plateacquisitions/106140/2