viewport URL includes z-plane

[will-moore]

Fixes #355

To test:

• Open a multi-z image in iviewer
• Scroll to a particular Z-plane (zoom in to feature of interest)
• Right-click on Image -> Get Link to Viewport. Copy the link (notice it includes z=...
• Paste the URL in new browser tab - this should take you to the specified Z plane (and location)

--exclude

[pwalczysko]

Works as expected. Ready to merge fmpov.

[jburel]

I don't think this is working as you expect, also what about T?

• log as user-1 ImageID: 1132

• select z=2 t=3

• 

• Select Get Link to Viewport

• URL contains z but not the correct value and of course no T

[will-moore]

I guess there's a decision to make about whether to store the actual Z index (0-based) or the UI displayed Z index (1-based).
I would vote for the 0-based index since it's closer to the truth. e.g. see last commit.
Actually I would prefer that the UI was also 0-based (as in napari, vizarr, icy etc) since the off-by-one pain is not worth it and just confuses everyone.
There are many situations where you are in a grey area between the UI and the server (e.g. in this case or when exporting ROIs/Shapes in a CSV) where you don't know if the Z/T indices are 0-based or 1-based.

[jburel]

Changing the UI is a breaking change and it needs to be considered carefully: not only in iviewer, but in the doc, video etc.

[jburel]

The feature now works as expected and T is also taken into account

[will-moore]

NB: Although the Z and T were never set in the URL before, it's possible that users were constructing their own URLs with Z and T. In which case, the usage of 0-based indexing here instead of 1-based indexing would be a breaking change.
However, this will likely have a much smaller impact to make the change now, instead of after we support the adding of Z and T to the URL (in this PR). And I think that the API should be using 0-based indexing.
Thoughts?

[jburel]

The possible problem is that the old viewer (that is still turned on by default) uses 1-based index
e.g. https://merge-ci.openmicroscopy.org/web/webgateway/img_detail/1132/?dataset=1051&z=0 will not work

[will-moore]

OK, so either we stick with 1-based index from now on. 👎
Or we also update the old viewer to use 0-based index. 👍
Always best to make a breaking change sooner (while the exposure is less) than later.
Votes?
cc @joshmoore

[sbesson]

Trying to look into where these conventions might have been documented (or not), I realized that the webgateway render API also uses 0-based indexing e.g. the following calls work

https://idr.openmicroscopy.org/webgateway/render_image/12922202/0/0/
https://idr.openmicroscopy.org/webgateway/render_image/12922202/44/0/
https://idr.openmicroscopy.org/webgateway/render_thumbnail/12922202/?z=0
https://idr.openmicroscopy.org/webgateway/render_thumbnail/12922202/?z=44

but the following calls will return a 404

https://idr.openmicroscopy.org/webgateway/render_image/12922202/45/0/
https://idr.openmicroscopy.org/webgateway/render_thumbnail/12922202/?z=45

Moving forward, unifying on 0-based indexes feels like the most consistent approach across the OMERO.web URLs. For the old viewer, there is definitely a breaking impact. Is this the only place that uses 1-based indexes for Z/T? Is there a way to enfore some compatibility e.g. via key/value pairs or options?

[joshmoore]

It's hot, so I may not be thinking very clearly but some points:

• I can certainly see the value of being 0-based everywhere.
• I worry that in many of these things, we're not in a position where we can make changes. I don't exactly know how to fix that, but it might be good to spend some time thinking about that root-cause. e.g. would changing from z to z_index (or whatever) alleviate potential breakage.
• More generally, what's the thinking on deprecating the old viewer?

[jburel]

I am for deprecating the old viewer. One of the problems is that the old viewer code is used in the right-hand panel in the main view. So you can explore the image without opening a viewer.
deprecating the old viewer will also mean the installing web will come without any viewer. All that need to be balanced carefully

[will-moore]

What does "deprecating the old viewer" mean for users? Remove it (no viewer)?
I think a webclient without a viewer would be considered incomplete.
So pip install omero-web should include iviewer by default, with all the config enabled already?

[will-moore]

So, should I update the old viewer to use 0-based indexes for z & t. e.g.
https://merge-ci.openmicroscopy.org/web/webgateway/img_detail/1132/?dataset=1051&z=0
??

[jburel]

that could break many people who have bookmarked the link with z=1

[will-moore]

It could. But do we want the possibility of bookmarks to prevent us from making any changes to our API?

An alternative is we use upper case keys for the recent 0-based indices ?Z=0&T=0 so we can distinguish between these and the previous 1-based ?z=1&t=1 URLs?

e.g. Django uses a more explicit naming to distinguish in templates:

forloop.counter | The current iteration of the loop (1-indexed)
forloop.counter0 | The current iteration of the loop (0-indexed)

But then we'd be expected to support 0-based and 1-based indices everywhere. Probably better to make a single breaking change?

[jburel]

What I am arguing is not any change to the API but the path to get there.
Before making any change, we should announce that we will change the indexing of the old viewer. We cannot include without given a warning

W

[will-moore]

Ok, so if we are going to announce a breaking change etc, do we also want to make the change in the UI (iviewer and old viewer) too, as I suggested above?
Then we are consistent everywhere and don't have to worry about off-by-one errors again?

EDIT: There's also OMERO.figure and iviewer if we want to be consistent everywhere.

[will-moore]

Just reviewing this PR again since the bug-fix that user requested is still not released.

Perhaps the solution is as Josh suggested: Use ?z_index=0 (zero-based index) for creating any new URLs but we maintain support for ?z=1 (1-based index) for existing URLs to avoid a breaking change. We can support this in both the iviewer and webgateway viewer.
The only down side is that it's not so concise, but that's a pretty minor issue, and you can't guess from the name that 1 is zero-based and the other is 1-based, but I think that's OK?

[joshmoore]

you can't guess from the name that 1 is zero-based and the other is 1-based, but I think that's OK?

Does z_offset help?

[will-moore]

Or, to indicate how this maps to the model, you could do ?theZ=0

[will-moore]

It would be good to get this fix merged (PR open 2 years now).

I would prefer to adopt ?Z=0&T=0 as zero-based indices going forward. This is the nicest solution and it's worth some extra work to get there.

For old bookmarks of the webgateway viewer, we can handle ?z=1&t=1 and notify users that they should now use zero-based ?Z=0&T=0.

Plan:

• Update this PR to use upper-case (zero-based) ?Z=0&T=0.
• Work on webgateway viewer to generate URLs with zero-based ?Z=0&T=0 and also to handle ?z=1&t=1 to notify users to update to use ?Z=0&T=0.

OK @jburel ?