Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Link url to image #584

Merged
merged 24 commits into from
Oct 23, 2024
Merged

Link url to image #584

merged 24 commits into from
Oct 23, 2024

Conversation

Rdornier
Copy link
Contributor

Fixes #514 by adding a KVP on each image used a the figure. The KVP consists on the Figure URL

@will-moore
Copy link
Member

A few flake8 errors in the build:

./omero_figure/views.py:373:39: E203 whitespace before ':'
./omero_figure/views.py:376:80: E501 line too long (116 > 79 characters)
./omero_figure/views.py:411:30: F821 undefined name 'rlist'
./omero_figure/views.py:411:80: E501 line too long (84 > 79 characters)
./omero_figure/views.py:414:26: F821 undefined name 'rlist'
./omero_figure/views.py:432:80: E501 line too long (101 > 79 characters)

NB: you should be able to use wrap( ["some string"] ) to wrap a list

@Rdornier
Copy link
Contributor Author

Rdornier commented Sep 2, 2024

Hi @will-moore

I fixed the flake8 errors.
I also fixed another bug to support the case when a figure already exists but the kvp is not yet created.

I finally noticed that BASE_WEBFIGURE_URL only contains the url from /figure/ and not from https://omero.server/webclient. Do you know how I can get this info ?

@will-moore
Copy link
Member

You can use https://docs.djangoproject.com/en/5.1/ref/request-response/#django.http.HttpRequest.build_absolute_uri
e.g.

url = request.build_absolute_uri(reverse('load_figure', args=[file_id]))
map_ann.setValue([["Figure_%s_%s" % (fig_name, file_id), url]])

@will-moore
Copy link
Member

Seems to be working fine from my testing so far...
I wonder if you could move that new code into a separate function(s), to keep the save_web_figure() function as simple/readable as possible.

@Rdornier
Copy link
Contributor Author

Rdornier commented Sep 3, 2024

Ok cool !

I wonder if you could move that new code into a separate function(s), to keep the save_web_figure() function as simple/readable as possible.

I agree, it could be better

I also noticed that I should remove the KVP from images when the figure is deleted, which is not yet the case.

@will-moore
Copy link
Member

I think it would be better not to include the figure Name in the Key because it's tricky to handle re-naming.
E.g. when I renamed a Figure and saved it, I got a new KVP annotation created, and the old one will be forever left on the images, even when they are removed from the Figure:

Screenshot 2024-10-03 at 11 48 53

Everything else seems to be working fine (delete Figure removes KVP etc).

@will-moore will-moore added this to the 7.2.0 milestone Oct 3, 2024
@Rdornier
Copy link
Contributor Author

Rdornier commented Oct 3, 2024

Thanks for your feedback.
Indeed, this is annoying but I would prefer to keep the name of the figure so that users knows in which figure the image is used, without having to open it.
I've made the fix in that direction.

Copy link
Member

@will-moore will-moore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, thanks.

@will-moore will-moore merged commit 24444fe into ome:master Oct 23, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add the link of the OMERO.figure if images are added to the figure
2 participants