-
Notifications
You must be signed in to change notification settings - Fork 54
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
Write scale less than 5D #114
Conversation
Codecov Report
@@ Coverage Diff @@
## master #114 +/- ##
==========================================
- Coverage 70.39% 70.10% -0.29%
==========================================
Files 11 11
Lines 1054 1104 +50
==========================================
+ Hits 742 774 +32
- Misses 312 330 +18
Continue to review full report at Codecov.
|
In response to the image.sc request above, I have tested this script locally:
After running that script, I can view in napari:
After playing with rendering settings: NB: also works in 3D view |
Made it through a first reading and some minor local testing. (Looks like I need to update my local installation. I'm getting black planes in napari!) One quick thought: would it make sense to add your |
Hmmm - I hadn't actually tested all the examples in
|
When I go into 3D view mode, the image and the labels align together, then when I return to 2D mode I see: |
To test:
Then:
NB: this needs ome/napari-ome-zarr#15 for |
This pull request has been mentioned on Image.sc Forum. There might be relevant details there: https://forum.image.sc/t/prepping-and-including-roi-masks/48750/20 |
That last commit should fix the issue that Khaled saw with the coins image initially rendered white in napari. |
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.
A few inline comments, probably outside the scope of this PR. Overall the proposed changes look reasonable to me and in agreement with the 0.3 specification. Once this has been reviewed across the various use cases (synthetic data, OMERO Zarr export, napari), I would propose to get this out as 0.2.0
.
@@ -19,22 +19,57 @@ def write_multiscale( | |||
group: zarr.Group, | |||
chunks: Union[Tuple[Any, ...], int] = None, | |||
fmt: Format = CurrentFormat(), | |||
axes: Union[str, List[str]] = None, |
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.
Possibly not an action for this PR but a general thought: as the specification gets refined and new concepts gets introduced (thinking concretely of the ongoing transformation
proposal), there might be a trade-off between adding every new key as an extra parameter vs e.g. passing some form of dictionary of extra metadata which will be validated depending on the specification.
""" | ||
|
||
dims = len(pyramid[0].shape) | ||
if fmt.version not in ("0.1", "0.2"): |
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.
While the axes guessing below definitely applies to 0.3, if the assumptions gets further relax, there will be an outstanding TODO of restricting this to the relevant versions of the specification.
The down-sampling of a 3D array in the form of 3D labels was tested by @pwalczysko at ome/omero-cli-zarr#82 (comment) |
@khaledk2 Yes, that is expected. Zarr won't overwrite the existing data that you created with the first export. |
I have tried |
@will-moore I have tested and it is working fine. |
Thanks all. Merging and going through the process of releasing |
Prompted by request for scripts to write masks data for napari: https://forum.image.sc/t/prepping-and-including-roi-masks/48750/17?u=will-moore
Since v0.3 now supports data with less than 5D, this should also be supported by the writing and scaling functionality.
This adds support for
scaler.nearest(data, x, y)
where data is 2D - 5D.Same for
write_image(data, scaler)
.Breaking change
write_image()
andwrite_multiscale()
MUST be provided withaxes
argument (except for v0.1 or v0.2) since this is required by the v0.3 spec.Also add 2D and 3D shapes to test_scaler and test_writer.
To test:
Then:
NB: this needs ome/napari-ome-zarr#15 for
napari
.