Simply download the OriginalFiles #264

dominikl · 2021-12-10T15:59:36Z

Fixes #188, successor of closed PR #217.

I'm very tempted to just keep it simple like this @jburel , ie. with this PR the download option will simply download the OriginalFiles and their directory structure. It won't create a separate directory or rename any files. If a file already exists, it will be skipped. If we make it more complex again, we'll run into all sorts of issues with special cases again.

/cc @ehrenfeu @CellKai

(Note: ~~Haven't tested it on Windows yet~~ Tested on Windows now too, incl. the latest commit)

src/main/java/org/openmicroscopy/shoola/env/data/OMEROGateway.java

pwalczysko · 2021-12-13T12:00:13Z

Deleted the comment, as I was able to download and unzip on subsequent attemps.

pwalczysko · 2021-12-13T15:17:47Z

Test 1:

Download one Dicom image from https://outreach.openmicroscopy.org/webclient/?show=image-13402
Reupload the downloaded image back to OMERO.

Result:
The image downloads as expected (it is a 3 image MIF, and the three images are downloaded locally).
Nevertheless, when the whole downloaded folder is reimported to OMERO, although the result are again 3 images, they do not appear to be multiimage file anymore, instead, for example, a Move into another group of only one of those images is possible.
Imho, this is a test fail.

dominikl · 2021-12-13T16:12:51Z

This also happens with the current Insight without this PR. Tbh, if download of the original files and reimport splits the multiimage Dicom image, than the problem is probably Dicom specific and lies deeper. Download and reimport of a vsi image for example works as expected with this PR.

pwalczysko · 2021-12-13T16:49:35Z

Test 2:

Download a vsi, using the insight from the present PR, namely https://outreach.openmicroscopy.org/webclient/?show=image-94204
Reimport it again to OMERO as https://outreach.openmicroscopy.org/webclient/?show=image-94208

Result: The downloaded folder structure seems reasonable, see screenshot below.
When the vsi file is picked and reimported to OMERO, the viewing in OMERO is as expected, and it is a MIF as expected.

This test passes imho.

pwalczysko · 2021-12-13T17:07:31Z

This also happens with the current Insight without this PR. Tbh, if download of the original files and reimport splits the multiimage Dicom image, than the problem is probably Dicom specific and lies deeper. Download and reimport of a vsi image for example works as expected with this PR.

Thank you @dominikl - confirming that with my release insight. The Dicom behaviour is not a regression, and has nothing to do with this PR.

joshmoore · 2021-12-13T17:16:12Z

If a file already exists, it will be skipped.

Would it be easy enough to expand this to "file or directory"? i.e. if anything already exists, you skip? I don't have a working example of how to trigger this, but I worried of downloading:

foo/my.ini

into a directory which already contains:

foo/my.fake

and thereby change the interpretation of something.

ehrenfeu · 2021-12-15T11:56:20Z

Hi there,

quick and hopefully not so stupid question... Should the changes from above also be included in omero-py available from Jenkins? I'd be curious to test them.

Thanks,
Niko

pwalczysko · 2021-12-15T12:45:59Z

Hi there,

quick and hopefully not so stupid question... Should the changes from above also be included in omero-py available from Jenkins? I'd be curious to test them.

Thanks, Niko

Hi Niko,

The changes are certainly available on https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-insight-build/ - just download the latest artifact of insight, that is what I am doing when testing. Not sure about omero-py, leave @dominikl to answer that.

dominikl · 2021-12-15T12:51:27Z

As Petr said, the changes are included in the current merge build of Insight. But they won't go into omero-py, that's totally unrelated to Insight.

dominikl · 2021-12-15T16:13:19Z

Last commit should address @joshmoore 's issue. Would be good to have some complex filesets (many files, many directories) to properly test this PR. What's a good image format for that?

pwalczysko · 2021-12-15T18:21:01Z

Last commit should address @joshmoore 's issue. Would be good to have some complex filesets (many files, many directories) to properly test this PR. What's a good image format for that?

Tested so far

vsi
ndpi
svs
Dicom (does not work, but not a regression)

The behaviour was as expected. @dominikl to find one complex file format which will test it all is possibly not realistic ? Could we specify (with examples) what the "if the file already exists, it will be skipped" means, or, if that changed as a result of your last commit, how exactly and what is the expectation ? Only based on that we can set up some testing cases.

ehrenfeu · 2021-12-15T21:18:23Z

As Petr said, the changes are included in the current merge build of Insight. But they won't go into omero-py, that's totally unrelated to Insight.

Thanks Dominik. I somehow thought omero-py is calling the Java code when downloading files, but I should actually know better...

will-moore · 2021-12-16T09:51:57Z

@ehrenfeu See https://forum.image.sc/t/download-full-projects-with-their-datasets/56434/22 and discussion there for downloading via python. Improvements were made in omero-py 5.10.0.

ehrenfeu · 2021-12-16T10:47:27Z

Ah, very nice - thanks a lot @will-moore !

I was asking for a friend working on the HRM-OMERO connector 😁 but he already found out that he should just properly deal with the paths when downloading filesets from OMERO.

Nevertheless I will look into your code. My parts in the connector are a bit aged by now, they could certainly use some polishing.

sbesson · 2021-12-16T10:48:59Z

Cross-linking to ome/omero-py#298 which added the Python improvements that @will-moore was referring too. This thread in particular includes many considerations around data integrity and data corruption that are directly relevant to this use case.

Adding to the similarities, the former CLI download allowed the user to specify the name of the output file breaking the assumptions of the file readers when doing a round-trip. This is effectively another variant of #188.

The agreement made in OMERO.py was to:

keep the structure of the original files (names & folder layout) unchanged
enforce the containerisation of a fileset under a top-level folder.

Talking with @pwalczysko @dominikl @will-moore and @khaledk2, I would strongly vote for using the occasion and be consistent in terms of the strategy across our download utilities.

Probably the biggest differences between omero download and OMERO.insight download are that OMERO.insight supports the export of multiple filesets and also that it does not allow control of the folder name by the user. In general, I think a export structure of type:

fileset1_folder/
   fileset1_originalfile_1
   fileset1_originalfile_2
   ...
   fileset1_originalfolder_1/
   ...
 fileset2_folder/
   fileset2_originalfile_1
   fileset2_originalfile_2
   ...
   fileset2_originalfolder_1/
   ...

is the one that is the most amenable to ensure the integrity of the original data and reduce the risks of cross-talk between filesets. In a nutshell, this is a small version of the ManagedRepository layout and matches the layout used in omero-downloader.

Probably the most important design decision for the layout above is the naming of these top-level fileset folders. There are a range of available options which offer trade-offs in terms of user-friendliness vs potential for interference with existing/other fileset folders.

joshmoore · 2021-12-16T10:55:02Z

cc: @sukunis in case there are any cross-interactions with openlink.

jburel · 2022-01-10T10:34:47Z

src/main/java/org/openmicroscopy/shoola/env/data/OMEROGateway.java

-					values.put(fs.getOriginalFile(), set);
+			for (Object tmp : files) {
+				Fileset fs = (Fileset) tmp;
+				File filesetDir = new File(dir.getAbsolutePath()+File.separator+"Fileset_"+fs.getId().getValue());


method like Files.createDirectories could be used here instead.

Doesn't really make it simpler, for Files.createDirectory I'd need to create a Path object instead of a File.

pwalczysko · 2022-01-10T13:50:56Z

Testing the behaviour of the repeated dowload of the same file into the same folder.
I have in Activities a message:
No image downloaded. Check logs for details.
Then in the log I have
2022-01-10 13:47:47,939 DEBUG [ o.o.shoola.env.data.OMEROGateway] ( Thread-46) /Users/pwalczysko/10Jan-test-download/Fileset_97307/Brain_DAPI_GFAP-488_IBA3-Cy3_MBP-647_coronal10x_coronal-slideA_03.vsi already exists.
This makes sense and is as expected, but could we not have the main message saying ..already exists in the Activities ? This would spare me having to go into the log - I have to know where the log is etc...

pwalczysko · 2022-01-10T17:19:32Z

Tested so far

vsi (also reimport)
ndpi
svs
Dicom (does not work in the sense that the MIF is decoupled after reimport, but not a regression)

The above went fine.

I think that the most logical to try now would be a plate download, but this si not enabled on merge-ci. What is the present workflow to change the config on that server please @sbesson ?

pwalczysko · 2022-01-11T12:49:02Z

Downloading https://merge-ci.openmicroscopy.org/web/webclient/?show=plate-4256 and https://merge-ci.openmicroscopy.org/web/webclient/?show=plate-2955 now.

The https://merge-ci.openmicroscopy.org/web/webclient/?show=plate-3855 is ending up being many single Filesets, possibly because it is a result of Dataset to Plate script ?

ehrenfeu · 2022-01-11T14:40:34Z

In case you're interested, I have created a very small (~200 kB) multifile VSI dataset using the original Olympus cellSens software here at our facility. My purpose is to use it for automated testing of the HRM-OMERO connector, hence I tried to have it as small as possible while keeping the multifile structure (once it gets too small, cellSens puts everything into a single .vsi file without any subfolders).

Let me know if I should upload it somewhere!

dominikl · 2022-01-12T16:00:03Z

@pwalczysko Unfortunately I can only log the error if the file already exists. At the moment it's not possible to pass an error message up to the UI level. That would need a lot of changes to the Insight code.

pwalczysko · 2022-01-12T18:23:05Z

The plates https://merge-ci.openmicroscopy.org/web/webclient/?show=plate-4256 and https://merge-ci.openmicroscopy.org/web/webclient/?show=plate-2955 downloaded nicely, and the reimport as https://merge-ci.openmicroscopy.org/web/webclient/?show=screen-47354 went fine too. I do not see any problem there.

pwalczysko · 2022-01-12T18:26:08Z

Any ideas for further testing formats @sbesson ?

Tested so far

vsi (also reimport)
ndpi
svs
Dicom (does not work in the sense that the MIF is decoupled after reimport, but not a regression)
xdce extension (Plate)
Mias Frans (Plate, I think this is Flex ?)

joshmoore · 2022-01-13T08:53:34Z

#264 (comment) possibly because it is a result of Dataset to Plate script ?

That would make sense, @pwalczysko.

sbesson · 2022-01-13T09:00:53Z

The list of formats tested in #264 (comment) should cover a wide-range of scenarios in different imaging domains (HCS, pathology). I assume NDPI included the NPDIS case (i.e. a pathology fileset composed of multiple NDPI files)?
https://docs.openmicroscopy.org/bio-formats/6.8.0/formats/dataset-table.html gives the full list of supported formats including the expectations in terms of dataset structure. The only other thing I can think of would be a multi-file format in the fluorescence microscopy domain. Deltavision, Zeiss LSM and/or Olympus FV1000 might be good examples.

pwalczysko · 2022-01-13T09:37:12Z

ndpi was https://merge-ci.openmicroscopy.org/web/webclient/?show=image-103615, which is just ndpi. I will try https://merge-ci.openmicroscopy.org/web/webclient/?show=image-176115 which is ndpis.

pwalczysko · 2022-01-13T15:43:16Z

In case you're interested, I have created a very small (~200 kB) multifile VSI dataset using the original Olympus cellSens software here at our facility. My purpose is to use it for automated testing of the HRM-OMERO connector, hence I tried to have it as small as possible while keeping the multifile structure (once it gets too small, cellSens puts everything into a single .vsi file without any subfolders).

Let me know if I should upload it somewhere!

Hi Niko,
Thank you very much for this. Would you be able to upload this file and make it publicly visible ? If you do not have a location, could you possibly use Zenodo ?
Thanks a lot
Petr

pwalczysko · 2022-01-13T16:20:35Z

zeiss-lsm see https://merge-ci.openmicroscopy.org/web/webclient/?show=image-210326
deltavision with a log file https://merge-ci.openmicroscopy.org/web/webclient/?show=image-210329
fv1000 (oif with multi tiffs to it) https://merge-ci.openmicroscopy.org/web/webclient/?show=image-210331

the three formats as above successfully downloaded and reimported. All MIFs and looked good after download and also after reimport.

This would be the coverage as suggested by comment #264 (comment)

We have now

vsi (also reimport)
ndpi and ndpis, reimported
svs
Dicom (does not work in the sense that the MIF is decoupled after reimport, but not a regression)
xdce extension (Plate), with reimport
Mias Frans (Plate, I think this is Flex ?), with reimport
fv1000, multifile oif variant, reimported
Deltavision with log, reimported
zeiss-lsm (multifile version with mdb), also reimported

As all files and functional tests passed and the coverage seems rich enough, ready to merge fmpov.
Happy to add more tests if someone comes with more format ideas, otherwise all good.

sbesson · 2022-01-14T09:52:30Z

Nothing else from my side. We might want to wait on the data from #264 (comment) to conduct a final test. Otherwise, I assume we are in a state where we should release OMERO.insight.

ehrenfeu · 2022-01-14T11:21:03Z

Hi @pwalczysko

here you go: https://doi.org/10.5281/zenodo.5848987

Thanks a lot!
~Niko

pwalczysko · 2022-01-14T12:09:11Z

@ehrenfeu Thanks a lot Niko !

Downloaded, imported to OMERO and again downloaded from OMERO using insight from this PR.
All went fine, the set consists of 2 images in OMERO, a 2-channel image with 11 z sections and other parameters matching the README of your file in Zenodo. The second image is Macro image. See screenshot below documenting how the hierarchy looks like on my local Mac after download and how the images look in OMERO.iviewer after reimport. I cannot detect any problem there, all works and matches the images pre-download.

joshmoore · 2022-01-17T08:07:16Z

cc: @erickmartins since he may have something similar in his upcoming transfer tools

https://twitter.com/erickratamero/status/1482060699539058688

erickmartins · 2022-01-17T14:48:42Z

We live in a python/ezomero world, so our strategy is to use our get_original_filepaths ezomero function (which HQLs for usedFiles in a Fileset) and rsync via subprocess maintaining the internal ManagedRepository directory structure. In our experience it has dealt with all cases pretty cleanly. If fileset downloading is included in omero-py we could potentially lose the rsync dependency and do it that way.

(we're probably making our transfer thing public this week. the repo is still private because we didn't want a half-half-baked solution in the hands of people, and because we still need a catchier name than jax-omero-transfer :) )

ehrenfeu · 2022-02-18T15:46:38Z

@pwalczysko @jburel sorry if I'm repeating myself, I feel like having asked this question before... Is there an automatic build of insight available somewhere that includes this merge?

jburel · 2022-02-18T15:50:07Z

I am preparing a release. It should be out next week. But you can try the daily build if you want. It is at https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-insight-build/. That builds does not have the dmg, exe so it is very much for testing purpose

ehrenfeu · 2022-02-18T15:51:40Z

Thanks J-M, highly appreciated!

jburel · 2022-02-21T14:14:18Z

@ehrenfeu https://github.com/ome/omero-insight/releases/tag/v5.7.0 containing this PR is now available

imagesc-bot · 2023-07-06T11:01:32Z

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/download-vsi-files-from-omero-fails/83314/2

Simply download the OriginalFiles

b3ce080

jburel reviewed Dec 13, 2021

View reviewed changes

src/main/java/org/openmicroscopy/shoola/env/data/OMEROGateway.java Outdated Show resolved Hide resolved

jburel reviewed Dec 13, 2021

View reviewed changes

src/main/java/org/openmicroscopy/shoola/env/data/OMEROGateway.java Outdated Show resolved Hide resolved

Use try-with and File.separator

4aaccd8

Check for existing/conflicting directories as well.

152e7ca

dominikl added 3 commits December 20, 2021 10:46

Download to Fileset_<ID>

7e9a183

Improve download activity messages

2f4aacd

Fix imports

93c5028

jburel reviewed Jan 10, 2022

View reviewed changes

dominikl mentioned this pull request Jan 14, 2022

Download popup "File format" chooser #271

Closed

pwalczysko mentioned this pull request Feb 4, 2022

Check whether multifile formats work as-is ome/omero-cli-transfer#4

Closed

pwalczysko mentioned this pull request Feb 17, 2022

Multifile ome/omero-cli-transfer#14

Merged

pwalczysko approved these changes Feb 18, 2022

View reviewed changes

jburel merged commit 3631862 into ome:master Feb 18, 2022

dominikl mentioned this pull request Jul 6, 2023

Olympus .vsi download bug ome/omero-gateway-java#77

Open

Rdornier mentioned this pull request Jul 7, 2023

Fix bug on download images based on omero-insight#264 ome/omero-gateway-java#78

Merged

dominikl mentioned this pull request Jul 10, 2023

Use Java gateway for downloading images #374

Open

Simply download the OriginalFiles #264

Simply download the OriginalFiles #264

Conversation

dominikl commented Dec 10, 2021 • edited Loading

pwalczysko commented Dec 13, 2021

pwalczysko commented Dec 13, 2021

dominikl commented Dec 13, 2021

pwalczysko commented Dec 13, 2021

pwalczysko commented Dec 13, 2021

joshmoore commented Dec 13, 2021

ehrenfeu commented Dec 15, 2021

pwalczysko commented Dec 15, 2021

dominikl commented Dec 15, 2021

dominikl commented Dec 15, 2021

pwalczysko commented Dec 15, 2021 • edited Loading

ehrenfeu commented Dec 15, 2021

will-moore commented Dec 16, 2021

ehrenfeu commented Dec 16, 2021

sbesson commented Dec 16, 2021 • edited Loading

joshmoore commented Dec 16, 2021

jburel Jan 10, 2022

Choose a reason for hiding this comment

dominikl Jan 12, 2022

Choose a reason for hiding this comment

pwalczysko commented Jan 10, 2022

pwalczysko commented Jan 10, 2022 • edited Loading

pwalczysko commented Jan 11, 2022 • edited Loading

ehrenfeu commented Jan 11, 2022

dominikl commented Jan 12, 2022

pwalczysko commented Jan 12, 2022

pwalczysko commented Jan 12, 2022

joshmoore commented Jan 13, 2022

sbesson commented Jan 13, 2022

pwalczysko commented Jan 13, 2022 • edited Loading

pwalczysko commented Jan 13, 2022

pwalczysko commented Jan 13, 2022 • edited Loading

sbesson commented Jan 14, 2022

ehrenfeu commented Jan 14, 2022

pwalczysko commented Jan 14, 2022

joshmoore commented Jan 17, 2022

erickmartins commented Jan 17, 2022

ehrenfeu commented Feb 18, 2022

jburel commented Feb 18, 2022

ehrenfeu commented Feb 18, 2022

jburel commented Feb 21, 2022

imagesc-bot commented Jul 6, 2023

dominikl commented Dec 10, 2021 •

edited

Loading

pwalczysko commented Dec 15, 2021 •

edited

Loading

sbesson commented Dec 16, 2021 •

edited

Loading

pwalczysko commented Jan 10, 2022 •

edited

Loading

pwalczysko commented Jan 11, 2022 •

edited

Loading

pwalczysko commented Jan 13, 2022 •

edited

Loading

pwalczysko commented Jan 13, 2022 •

edited

Loading