Fix breaking change: _marshal_images() handles missing archived flag

[will-moore]

The change at https://github.com/ome/omero-web/pull/555/files#diff-dfcabd11e666e142b223e6aee67714ee9badf6ecdcaa5ceec24b7805e3d49520R553
has caused apps that use this to break:

• mapr Archived status in _marshal_image() omero-mapr#84
• auto-tag https://www.openmicroscopy.org/qa2/qa/feedback/41954/

This fix handles the missing archived flag (where row has 5 items) and defaults it to false.

To test:

• Test with auto-tag (load central panel) or with mapr (search/browse by KVP in left panel > Project > Dataset > Images to load images in tree and centre pane)

Propose to release this fix ASAP (as 5.27.1)?

[sbesson]

At least from a code review perspective, the proposed change should provide a layer of compatibility for any downstream code using the signature of omeroweb.client.tree._marshal_images prior to OMERO.web 5.26.0. Functional testing should ensure that the archived status of the image is still properly exposed as this was the primary motivation of #555.

From a communication perspective, I would be mindful with the usage of breaking. At least I use this terminology as defined in https://semver.org/ i.e. in the sense of backward incompatible changes made to the public API.
In the case of this change:

• the reference documentation classifies the entire webclient module as non-public API - see note at https://omero.readthedocs.io/en/stable/developers/Web/Webclient.html
• the method itself starts with an underscore, indicating a weak internal use as per https://peps.python.org/pep-0008/

In that sense, there is probably a wider question about why these applications are using this internal method of a private API in the first place and whether there would be some proper public API (e.g. the marshalled object returned for api/v0 that could be proposed as a suitable replacement.

[will-moore]

In the case of mapr, this is using the JSON response to populate the jsTree, which is also using JS code from the webclient's jsTree, so it makes sense to use the same python function to marshal the data.

[knabar]

In the case of mapr, this is using the JSON response to populate the jsTree, which is also using JS code from the webclient's jsTree, so it makes sense to use the same python function to marshal the data.

Agreed that this makes sense and avoids the client having to implement the same method again, but we should then at least promote the _marshal_images method to something more public, at a minimum removing the underscore.

[knabar]

The deprecation warning should explicitly state how to fix the deprecated usage, e.g. "row argument now requires additional 'archived' entry at index 1"

[chris-allan]

This is at best unfortunate and when I made the original changes I made them the way I did because they were in webclient and the functions were underscore prefixed. These changes certainly cannot go in without expanding the test cases that were adjusted to handle #555.

While I'd be one of the first to say "breaking a downstream plugin is always a bug" fixing it like this feels really uncomfortable. My first thought is to revert #555 entirely and approach things differently if we're going to start saying things in the omeroweb.client.tree package are public API. Having tree derivatives miraculously not show the archive flag because they happen to be plugins is highly not ideal. I'll give it some more thought.

[will-moore]

Closing in favour of #577