ZarrReader: Update to support 0.4.0 datasets #8
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi David, |
The workflow is:
|
|
Following the workflow #8 (comment) I get following error: This error I get after I have already removed the OMERO plugin and the simple Macro plugin. Previously, the report about the conflicting files had five lines (now it has only 3, but the conflict still persists) |
|
With freshly downloaded Fiji:
|
|
In summary, seems to work, but questions
|
|
I don't know how to test this. Followed #8 (comment) . You can't select a directory, I guess that's why "Select a file inside the 0 subfolder of your local zarr", i.e. you'd have to go down the whole directory tree |
|
@pwalczysko : I've merged #9 into this PR which means that we should have a testable jar if we want to try to improve/capture new testing instructions. |
|
Can try tomorrow. This requires a newly downloaded Fiji, which would take very long from home. |
|
Whilst retesting with the github actions workflow, encountered probs:
|
|
@joshmoore see updated workflow
|
pwalczysko
suggested changes
Nov 10, 2021
joshmoore
added a commit
to joshmoore/ZarrReader
that referenced
this pull request
Nov 26, 2021
By adding the testSourceDirectory, the `mvn test` target can be used in intellij to run tests. This detected an NPE on `axes` that I've fixed here though it would be more appropriate for omegh-8.
joshmoore
added a commit
to joshmoore/ZarrReader
that referenced
this pull request
Nov 26, 2021
By adding the testSourceDirectory, the `mvn test` target can be used in intellij to run tests. This detected an NPE on `axes` that I've fixed here though it would be more appropriate for omegh-8.
Merged
|
@pwalczysko : do you remember where you got your 6001247 from? |
From local, certainly downloaded as a part of other testing, see below. |
|
Downloaded from where? Which NGFF version 0.2? Is there a "dimension_separator" flag in the .zarray files? I created a new one using the latest omero-cli-zarr and the file loads in napari correctly. The current ZarrReader test passes with these changes: and found non-zero values in all images: |
|
I think the version is to be gleaned from my |
|
Could you attach here the output of: Also, which version of napari and the napari-ome-zarr plugin? |
@joshmoore see above as well as my exact testing setup is actually described in the header of ome/omero-cli-zarr#82 (comment)
Edit: To answer the question about napari version more explicitly |
|
With the which leads to this exception in OMERO: |
|
I assume this comes from |
|
What's the next step here? As discussed on Monday, adding support for the imminent 0.4 specification is high on the priority list. Should we get this merged and open a follow-up PR for adding support and we can carry-out with the OMERO cross-version testing? Or are there some outstanding blockers that should be resolved first? |
|
Happy to get this in if that unblocks other things but import is still failing with a NPE on today's build: and A few other things I noticed for anyone else who wants to test:
|
dgault
changed the title
|
Updated with parsing metadata for the new axes and coordinateTransformations. The axes parsing should also be backwards compatible with v0.3 Ive also removed the top level zarr folder from usedFiles to address |
|
So (fun never ceases) without the top-level directory in the setId, the upload in OMERO drops the which leads to: I think Bio-Formats is doing what it can to have the paths all correct: but OMERO's logic is to remove all paths which are redundant. Options I can think of include:
|
|
@melissalinkert reminded me of another option |
|
I tested this reader against different datasets:
What is the easiest way to collect these and prioritize the fixes? Happy to see this merged and start collecting the failures as separate issues to review. Or have a more verbose digest of the samples and the exceptions on this PR. In the meantime, I will start working on combining a collection of reference OME-NGFF 0.4 samples. |
|
Tested locally and with FIJI/QuPath for all the datasets in The issue with blank planes seems to be limited to OMERO and Im still not entirely sure of the cause. My assumption is that it is likely related to the paths being stored, so the second commit aims to ensure only the zarr paths are being stored. |
|
Retested a full OMERO import workflow and loading of sample data like the ones of https://www.openmicroscopy.org/2021/12/16/ome-ngff.html still fails post metadata import with the following exception of I separately worked on testing the same datasets locally using the new reader. On my M1 system, I had to work around an issue with the JNA library support for ARM64 following the instructions of Philippus/sbt-dotenv#91 (comment) and setting Running a simple command-line was sufficient to open the image and display the planes without issue. Tested various combinations of arguments to reproduce the OMERO stack ( A potential mismatch can be identified by passing the while in flattened resolution mode, the series are been downsampled down to 64. So there might be some issue with the sub-resolution logic? |
|
Yeah, looks like the sub-resolution logic got broke somewhere along the line. Fixes in the latest commit should resolve those issues |
|
Thanks, with the last commit, running returns the smallest resolution as expected and the metadata looks good. Importing the image into OMERO still the same So the issue is related to tiled reading. I can reproduce the issue in command-line works but throws an A possibly related issue is that Bio-Formats detects |
|
Retested the last commit by importing several OME-NGFF samples into test servers with this JAR included
Overall, I think we have a working proof of concept and I would suggest we get this merged and tagged. The testing also flagged several usability/performance/support issues which I would propose to capture as separate issues so that they can be addressed individually:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some minor changes to support metadata from 0.3.0 based on https://ngff.openmicroscopy.org/latest/#metadata
Also some handling of datasets that don't have a 5d shape in .zarray
This should support reading of datasets in 0.3.0 and backwards compatibility should be maintained.