-
Notifications
You must be signed in to change notification settings - Fork 34
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
Initial OME Zarr HCS implementation #69
Conversation
--no-hcs falls back to the original ```series/resolution``` group name format.
...and update the converter to make them pass.
In particular, removes leading plate index.
plateMap.put("name", meta.getPlateName(plate)); | ||
|
||
List<Map<String, Object>> columns = new ArrayList<Map<String, Object>>(); | ||
for (int c=0; c<meta.getPlateColumns(plate).getValue(); c++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After 30min of conversion of a plate, the call ended with a java.lang.NullPointerException
while trying to write the HCS metadata. Now I understand better why ome/bioformats#3638 is required :)
Two questions:
- does this mean progress is blocked on having Populate plate row and column dimensions ome/bioformats#3638 available in an mainline release?
- or do we need to implement some fallback anyways for the readers where this metadata might be lacking?
Tested the last commit with the IDR Bio-Formats JAR and A few comments:
|
The metadata contains 0-based indexes and optional
Not really, unfortunately. I run the bioformats2raw JSON through a pretty-printer (https://github.com/makamaka/JSON-PP) as needed, but that doesn't completely solve the problem. |
Which IDR Bio-Formats commit specifically were you using? I am having trouble finding a version that detects 6 fields and plate acquisitions without throwing an exception. |
Sorry, I should have precised, I build
and the conversion command I used was
|
Ah, OK. That's the same version I was using, but a different dataset (I had https://downloads.openmicroscopy.org/images/Flex/idr0001/). As far as I can tell by looking at the original data for |
After double checking the data, you are absolutely right the dataset contains multiple fields of view per well for each acquisition
Unfortunately this means the dataset is not directly comparable to the data exported from IDR as only the first field of view of each well seems to have been imported. Proposing to discuss at the next Formats meeting the next steps for validating this PR as well as the handling of empty rows/columns. |
@sbesson: We would like to get this in for the forthcoming 0.3.0 release. Is there anything further you'd like to discuss/test here beforehand? |
Conflicts resolved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chris-allan thanks for the heads up.
Summarizing briefly, most of my testing has been focused on the use case of plates with multiple acquisitions. This is primarily because this was the scenario we were trying to work in terms of specification. Unfortunately for the reasons discussed above, it is not straightforward to compare the output of bioformats2raw vs omero-cli-zarr.
A potential test might to try and convert the other sample plates discussed in the public OME-Zarr HCS spec blog post using bioformats2raw
and compare the output with the public Zarr representations on S3. Given the preliminary testing as well as my current capacity I certainly don't feel a compelling reason to block the upcoming minor release though.
Instead I'd propose to schedule this conversion test using bioformats2raw 0.3.0
and capture issues separately.
@melissalinkert: This PR is ready to have conflicts resolved prior to merge. |
Conflicts resolved. |
See ome/omero-ms-zarr#75
Automatically uses HCS layout if a plate is present in the reader's
MetadataStore
, unless the--no-hcs
option is used.