ome / omero-figure

An OMERO.web app for creating Figures from images in OMERO
http://figure.openmicroscopy.org
GNU Affero General Public License v3.0
15 stars 30 forks source link

Remove id from image link description #339

Closed will-moore closed 4 years ago

will-moore commented 4 years ago

This uses "Image:123" instead of "Image ID: 123" in the description of images created from figures. As suggested (and supported) in https://github.com/ome/openmicroscopy/pull/6087#issuecomment-517166063

NB: That PR is not merged just now, so links aren't rendered, but layout looks cleaner:

Screen Shot 2019-08-30 at 11 17 41

joshmoore commented 4 years ago

NB: That PR is not merged just now, so links aren't rendered, but layout looks cleaner:

Similar to https://github.com/ome/scripts/pull/152#issuecomment-525635552, we need to consider how this kind of statement will be communicated to users. e.g. someone who updates figure without updating the related server version will lose their links.

will-moore commented 4 years ago

The rendering of Image: 123 to html link is not an essential feature from OMERO.figure point of view. The adding of Image references to the Description field was more about keeping track of the data provenance. So that will be the focus of the Figure release announcement.

The rendering of links has not previously been supported in webclient, so no one will lose their links by not using https://github.com/ome/openmicroscopy/pull/6087.

joshmoore commented 4 years ago

The rendering of Image: 123 to html link is not an essential feature from OMERO.figure point of view...

Fair enough.

The rendering of links has not previously been supported in webclient, so no one will lose their links

Ok. But perhaps a surprise for insight users. Mostly, I'm trying to stress that for any of these loosely-coupled dependencies (scripts, text-based formats, etc) we need to have solid strategies for dealing with breaking changes.

jburel commented 4 years ago

The space is not compliant with what was introduced a while ago when projecting an image from insight Insight will not be able to currently handle the link without some adjustments If we go for no space, we need to handle both case.

will-moore commented 4 years ago

OK, I guess I'm fine with it either way. Image:123 is nice because it's slightly more concise (easier to remember and not make typos) and the same format is supported in CLI etc. Does support for that need back-porting to the "deprecated" Insight?

jburel commented 4 years ago

The fact that it unifies the passing of argument is a valid point CLI, figure etc. will then follow the same pattern. This means that an adjustment should be made in insight and we will have to handle both space/no space in the UI

jburel commented 4 years ago

See insight adjustment https://github.com/ome/omero-insight/pull/78

will-moore commented 4 years ago

webclient PR to render links is now available for testing https://github.com/ome/omero-web/pull/16

jburel commented 4 years ago

tested against https://merge-ci.openmicroscopy.org/web/webclient/?show=image-91366 with and without ID in the description