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

v0.3 support #103

Closed
joshmoore opened this issue Jul 12, 2021 · 10 comments
Closed

v0.3 support #103

joshmoore opened this issue Jul 12, 2021 · 10 comments

Comments

@joshmoore
Copy link

cc: @will-moore @constantinpape @tischi @manzt

@joshmoore
Copy link
Author

Sorry, should have filed this earlier. @constantinpape is driving forward on v0.3 and it would be good to know one of:

  • how much time (calculating in holidays) a vizarr release might take? and/or
  • how explosive accessing v0.3 data with current vizarr's might be?

~J.

@will-moore
Copy link
Collaborator

With a tiny fix in isNested() to handle the missing omero metadata in that sample data:

diff --git a/src/io.ts b/src/io.ts
index 08858f7..2071851 100644
--- a/src/io.ts
+++ b/src/io.ts
@@ -120,7 +120,7 @@ function isNested(attrs: Ome.Attrs) {
   let version;
   if ('plate' in attrs) {
     version = attrs.plate.version;
-  } else if ('omero' in attrs) {
+  } else if ('multiscales' in attrs) {
     version = attrs.multiscales[0].version;
   } else if ('well' in attrs) {
     version = attrs.well.version;

the data is visible in vizarr:

Screenshot 2021-07-12 at 12 36 46

but, that data is chunked in 3D (instead of 2D as expected by vizarr), so we get some unexpected behaviour when trying to scroll through Z.

@joshmoore
Copy link
Author

With a tiny fix in isNested() to handle the missing omero metadata in that sample data:

❤️ Can that be a PR as well?

@manzt
Copy link
Member

manzt commented Jul 12, 2021

Looking through the links, it's not totally clear to me what needs to be added to vizarr to support v0.3. I guess now we can check if axes are present, and if the last two aren't y & x raise a more helpful error message.

Something I need to think more deeply about is the variable dimension handling. Fortunately vizarr doesn't assume 5D, but some of the logic in the OME- loaders might assume this. We can just inspect the axes key labels now (if v0.3) rather than assuming always tczyx.

3D chunking is still an issue and something we are thinking about. Adding support for volumetric rendering will likely entail extending the Tile 3D Layer from deck.gl. I should look into how other tools (like neuroglancer and napari) choose to render 2D multiscale from 3D downsampled datasets.

As a side note, I notice that the example data does not have omero metadata in the root attrs. It is my impression that this is required metadata for OME-Zarr, is that not the case?

❤️ Can that be a PR as well?

My hope is to just bump to the latest version of Zarr.js (with support for dimension_separator), and ideally move away from modifying the store interface depending on the ome metadata present. I can open a PR and let me know what you think.

@will-moore
Copy link
Collaborator

Sorry - just started on this already... #104

@joshmoore
Copy link
Author

As a side note, I notice that the example data does not have omero metadata in the root attrs. It is my impression that this is required metadata for OME-Zarr, is that not the case?

No. And in fact that's what I was referring to with #103 (comment) (a general 👍 on the separate nested/flat PR(s))

@manzt
Copy link
Member

manzt commented Jul 13, 2021

No. And in fact that's what I was referring to with #103 (comment) (a general 👍 on the separate nested/flat PR(s))

Ah, sorry I missed reading that. My mistake. Vizarr probably needs some data-driven heuristic like napari to "guess" initial contrast limits for the sliders if rendering metadata isn't present. It's fallback is the full range of values for the dtype, which is not desirable (except for |u1 where 0-255 is reasonable).

ref (data is lowest resolution in image pyramid): https://github.com/napari/napari/blob/1b9a7210cacea93db7299fd0beb52108d20a1ca0/napari/layers/utils/layer_utils.py#L35

@manzt
Copy link
Member

manzt commented Aug 12, 2021

Thanks for taking the lead on the changes @will-moore!

@joshmoore
Copy link
Author

@manzt : we are getting close to merging the various PRs on the Python side. How are you feeling about a release here? Shall we try to coordinate the process?

@manzt
Copy link
Member

manzt commented Aug 13, 2021

I'm happy to cut a corresponding v0.3 release for vizarr, that sounds great.

@manzt manzt closed this as completed Sep 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants