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

Consider making an update site #41

Open
NicoKiaru opened this issue Nov 8, 2022 · 10 comments
Open

Consider making an update site #41

NicoKiaru opened this issue Nov 8, 2022 · 10 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@NicoKiaru
Copy link

Hello @ppouchin,

Do you plan to make an update site with the OMERO simple client ? I know the installation is easy with a single jar, but with an update site it's possible to install it from the command line, and this would make my life easy with an installer script I wrote:

https://github.com/BIOP/biop-bash-scripts/blob/main/install_fiji_update_sites.sh

Thanks again for this super nice repo!

@NicoKiaru
Copy link
Author

Also, if there's a new version released, the update site can deal with removing the obsolete version.

@lacan
Copy link

lacan commented Nov 8, 2022

As a note, if the OMERO 5.5-5.6 update site is enabled, all the dependencies that simple-omero-client needs are downloaded. This means that there is nothing to add except the simple-omero-client jar file, if you decide to make an update site.

@ppouchin
Copy link
Member

ppouchin commented Nov 8, 2022

Hi @NicoKiaru, @lacan

I'm actually planning on making an update site, but I'm very slow...
The update site would include our two other plugins (macro extensions and batch processing), unless it is a bad idea?

I wanted to add methods to ease dependencies installation for end users (through a pop-up), as mentioned here (back in September...), but I've been busy with other requests in my lab the last couple months. Also, I'm not sure this code is appropriate.

@ppouchin ppouchin self-assigned this Nov 8, 2022
@lacan
Copy link

lacan commented Nov 8, 2022

There is no issue with having a bunch of things in the update site, we had started with one update site per project but it quickly became tedious to do.

We use our own server to host our files and connect through the Fiji updates via SFTP when we make updates:

image

but you can request an update site from the fiji wiki, which means you don't have to host the files yourself
https://imagej.net/update-sites/setup

@ppouchin ppouchin modified the milestone: 5.9.2 Jan 18, 2023
@ppouchin
Copy link
Member

Ok. I've finally updated the library (had not done so in a long time), but I now wonder if some refactoring is not required.
I'd at least remove "Generic" from some class names, but I may also rework the "addTag" method to check if the tag already exists (if it's possible). Having methods to browse the graph easily would be nice too (but if it's only an addition, it could wait later).
Breaking the API after putting it on the update site would be troublesome, wouldn't it?

I'll ask for the creation of an update site on the forums: would something like "Simple OMERO scripting" be a good name?
Or maybe just "Simple OMERO"? This last one could be confusing (but the first one too, maybe?).

If I try to make a version 6.0.0 (although it would be more of a version 2.0.0) and upload it before January 31st, would it be acceptable? (I'm already months late though...)

@NicoKiaru
Copy link
Author

Breaking the API after putting it on the update site would be troublesome, wouldn't it?

Well it's a problem independent from the update site, no ? I mean, changing API happens (https://forum.image.sc/t/trackmate-v7-10-0-released-new-tracker-new-gap-closing-algo-and-breaking-api/76095)

I'll ask for the creation of an update site on the forums: would something like "Simple OMERO scripting" be a good name? Or maybe just "Simple OMERO"?

Both look fine to me!

If I try to make a version 6.0.0 (although it would be more of a version 2.0.0) and upload it before January 31st, would it be acceptable? (I'm already months late though...)

Or course, our stuff is already working and it was just a request to simplify my life! And if one day you have the recipe for not being a few months late, late (sic) me know because I would need it badly.

@ppouchin ppouchin added this to the 6.0.0 milestone Jan 19, 2023
@ppouchin ppouchin added the enhancement New feature or request label Jan 19, 2023
@ppouchin
Copy link
Member

ppouchin commented Jan 27, 2023

So this should end up on the "OMERO" Fiji update site (soon).
Before that, I'd like to rewrite the API (which is ongoing), but I'm wondering if this makes sense:

  • I've added public interfaces and renamed some classes (removed "Generic" from classes names).
  • The interfaces for objects share the same name as they do in the OMERO model (e.g. TagAnnotation).
  • I've split the Client components in 4 interfaces: ConnectionHandler, Browser, DataManager and AdminManager
  • I'd like to add "getMapAnnotations" methods to the Browser interface.
  • I also should change the "addTag" methods for Add tag #31.
  • "getParents" and "getChildren" methods for repository objects would be nice as well, although images can have different types of parents (WellSamples / Datasets).
  • Maybe I should rename the "TableWrapper" method as "TableBuilder" and create a true "TableWrapper" inheriting from "ObjectWrapper", although it may not be critical.
  • A static map / method to automatically wrap DataObjects could be useful too...

The last Github Action generated the following Javadoc, but I may make the "Rectangular" interface an extension of "Shape" instead.

Would these changes be too much?

@ppouchin
Copy link
Member

ppouchin commented Feb 8, 2023

Hi @Rdornier , @NicoKiaru

I've changed quite a few things (in my development branches), and most notably improved performance for saving ROIs: our code used to only save ROIs one by one instead of saving them all at once (saving 1000 one by one takes 1 minute, while it takes 3 seconds to do them all at once).
EDIT: I've released a quick-fix for that with version 5.10.0

I still need to rework the Folders a bit, because I noticed it was not properly handled, and while I'm there, I'd like to improve the screen/containers hierarchy.

For that, I'd like your opinion, if possible. I wanted to have "getParents/getChildren" methods, but I find it hard to implement, (because of the "fixed" nature of the structures), so would it be acceptable to have two interfaces (for screens and containers) that declare methods to retrieve all the objects from any level linked to the current object?

What I mean by this is that I could make a "Container" interface, implemented by Projects, Datasets and Images, which would give them all: getProjects(), getDatasets() and getImages() methods (which is already mostly the case in fact).

The problem would then be how to handle "same level" objects (like calling getDatasets() from a Dataset): it can be handled by throwing an exception, returning a singleton (or empty) list, or returning a list of "cousin" datasets, but the last one would be weird. For Images, getImages could return the list of images from the same fileset, but even that could be odd.

Similarly, an "HCS" interface, implemented by Screens, Plates, PlateAcquisitions, Wells, WellSamples and Images (again) could be declared.

I'm also thinking of refactoring the packages, to better mirror the OME model: https://github.com/GReD-Clermont/simple-omero-client/tree/refactor-packages/src/main/java/fr/igred/omero

@Rdornier
Copy link
Contributor

Rdornier commented Feb 9, 2023

Hello @ppouchin,

Great to see simple-omero-client evolving !

so would it be acceptable to have two interfaces (for screens and containers) that declare methods to retrieve all the objects from any level linked to the current object?

I do not see anything that goes against. I think it is indeed a good idea to have directly access to parent project / screen from the image level.

The problem would then be how to handle "same level" objects

Yes, I think returning the object itself could be good. For images, returning the images within the filset is indeed a bit odd. For that case, I would rather add a method getFilset() in the imageWrapper class

In this implementation way, Images will have to implement "HCS" and "container" interfaces.

  • Both interfaces will implement a getImages() method. Doesn't this raise a conflict in the Image class ?
  • For images, how do you know an image has a dataset or a well as parent ? Should we test the method getDataset(), look at the results and if it returns an empty list, then we test the getWell() method ? Or is it something that is embedded in the Image class (like a hasDatasetParent boolean or checkParent() method) ?

I'm also thinking of refactoring the packages, to better mirror the OME model: https://github.com/GReD-Clermont/simple-omero-client/tree/refactor-packages/src/main/java/fr/igred/omero

I just have a comment on that. I do not really understand why the "core" folder in named like that for all classes dealing with images and image-related objects. As you said, it is mirrorred from the OME model but I would rather put "images" instead.
But this is just a naming convention and it doesn't impact much on the implementation.

Regarding your post from Jan.27, everything looks good to me. I just wonder if API changes and client split will affect too much the simple and easy usage of handling data, which is a strength of simple-omero-client.

I'd like to add "getMapAnnotations" methods to the Browser interface.

What is the purpose of getting them from the browser interface ? Is it to find all keys that have already been used to avoid creating an identical key but rather to use an existing one ? I unterstand that getTag() are implemented in the browser because tags can be shared within a group. But is that also the case for mapAnnotations ?

Before that, I'd like to rewrite the API [...] Would these changes be too much?

I think the changes are ok as long as you release them under 6.x.x version because it may break all scripts based on the previous release.

The last Github Action generated the following Javadoc, but I may make the "Rectangular" interface an extension of "Shape" instead.

The link on "javadoc" points to a zip file to download. Is that intentional ?

@ppouchin
Copy link
Member

ppouchin commented Feb 9, 2023

Thank you for your reply!

The last Github Action generated the following Javadoc, but I may make the "Rectangular" interface an extension of "Shape" instead.

The link on "javadoc" points to a zip file to download. Is that intentional ?

Yes, I uploaded an attachment to the comment, but I've now put the API for my latest experiment on the API website:
https://api.igred.fr/simple-omero-client/6.0.0-SNAPSHOT/

Regarding your post from Jan.27, everything looks good to me. I just wonder if API changes and client split will affect too much the simple and easy usage of handling data, which is a strength of simple-omero-client.

In terms of use, it should remain the same. There's only 1 concrete class GatewayWrapper, that implements a Client interface merging the different aspects. I've just split the methods between different interfaces because the file was getting quite big, and in this way, we have a vague idea of what kind of operation we're doing on the server. The interfaces contain implementation code because multiple inheritance is not possible in Java.

If someone wants to implement these interfaces in classes wrapping Gateway facilities, it is possible too.
But as it is, the main difference is the initialization: Client client = new GatewayWrapper(), otherwise it remains the main point of entry.

I do not see anything that goes against. I think it is indeed a good idea to have directly access to parent project / screen from the image level.

These considerations come, for me, from the development of the macro extensions for ImageJ: list(childType, parentType, id) cannot be handled in an easy/readable way, as I have to check which object I have and which method will be used for the class, it's not Object-Oriented at all and requires many switch.

Yes, I think returning the object itself could be good. For images, returning the images within the filset is indeed a bit odd. For that case, I would rather add a method getFilset() in the imageWrapper class

FYI, there's actually a getFilesetImages() since 5.9(.1).

In this implementation way, Images will have to implement "HCS" and "container" interfaces.

  1. Both interfaces will implement a getImages() method. Doesn't this raise a conflict in the Image class ?
  2. For images, how do you know an image has a dataset or a well as parent ? Should we test the method getDataset(), look at the results and if it returns an empty list, then we test the getWell() method ? Or is it something that is embedded in the Image class (like a hasDatasetParent boolean or checkParent() method) ?
  1. As these will be purely abstract interfaces, there's no conflict. Image concrete classes (e.g. ImageWrapper) will have to implement this method to satisfy both interfaces.
  2. ImageWrapper already implements both getWells() and getDatasets() and simply return empty lists if none is linked.

Finally, I'm not sure how to name these interfaces (HCSLinked?), but Annotation(Wrapper) could also implement both: in practice, we already have methods to retrieve all these objects. If you have a Tag, you can already use getWells() and getDatasets(), so saying they're linked to both types of hierarchy is true.

We'd thus have 3 interfaces:

  1. "ImageLinked": for all objects linked to an image (directly or through an HCS/Container tree), e.g. Folders, Tags, Screens, Projects, ...
  2. "ContainerLinked": for all objects linked to the container hierarchy (Projects/Datasets), e.g. Tags, Projects, Datasets and Images
  3. "HCSLinked": for all objects linked to the HCS hierarchy, e.g. Tags, Images, Screens, Plates, PlateAcquisitions, Wells and WellSamples

I'm wondering if these interfaces even need to be generic (by inheriting from RemoteObject to return the DataObject they contain) or not...

I'm also thinking of refactoring the packages, to better mirror the OME model: https://github.com/GReD-Clermont/simple-omero-client/tree/refactor-packages/src/main/java/fr/igred/omero

I just have a comment on that. I do not really understand why the "core" folder in named like that for all classes dealing with images and image-related objects. As you said, it is mirrorred from the OME model but I would rather put "images" instead. But this is just a naming convention and it doesn't impact much on the implementation.

I'll think about that. It would make sense.

I'd like to add "getMapAnnotations" methods to the Browser interface.

What is the purpose of getting them from the browser interface ? Is it to find all keys that have already been used to avoid creating an identical key but rather to use an existing one ? I unterstand that getTag() are implemented in the browser because tags can be shared within a group. But is that also the case for mapAnnotations ?

I think any kind of "Annotation" can be shared in Read-Annotate groups. For me, it's mostly to find all MapAnnotations which contain a given key, or a specific key-value pair, that can then be used to retrieve other objects (for example, using getProjects().

Before that, I'd like to rewrite the API [...] Would these changes be too much?

I think the changes are ok as long as you release them under 6.x.x version because it may break all scripts based on the previous release.

The aim is indeed to make it 6.0.0!

@ppouchin ppouchin mentioned this issue Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants