Add axes field to multiscale metadata #46

constantinpape · 2021-04-26T09:15:01Z

Add the axes field to multiscales and update latest to v0.2. Pending #44 and release of 0.2 (that's why I marked it as a draft).

The discussion whether axes is mandatory (MUST) or encouraged (SHOULD) is still pending. cc @joshmoore @sbesson @jni

joshmoore

Few thoughts (sorry if you're still in the process of updating)

latest/index.bs

constantinpape · 2021-04-26T09:56:37Z

I synced the latest changes to 0.2 now and updated all version numbers (I could find) to 0.3 now.

constantinpape · 2021-05-05T12:51:22Z

When migrating #39 I forgot to update the dimensionality requirements, i.e. allowing for image data with less than 5 dimensions.
I added this in the last commit.
@joshmoore let me know if you think we should formulate this more explicitly.

thewtex

👍 @constantinpape explicit dimensions are helpful to make the dimension identity self-evident and make the format easy to understand and use.

In the interest of compatibility with xarray, we could update the way the axes are encoded with:

        "0/example/.zattrs": {
            "_ARRAY_DIMENSIONS": [
                "t",
                "c",
                "z",
                "y",
                "x", 
           ],

per https://xarray.pydata.org/en/stable/internals.html#zarr-encoding-specification

thewtex · 2021-05-06T01:54:37Z

latest/index.bs

-    │   └── t.c.z.y.x         # a "chunk coordinate", where the maximum coordinate
-    │                         # will be `dimension_size / chunk_size`.
+    │   └─ t                  # Chunks are stored with the nested directory layout.
+    │      └─ c               # All but the last chunk element are stored as directories.


Can we stick with a flat directory? The motivation is compatibility with xarray and avoiding unnecessary overhead with IPFS 👾

Hi @thewtex. Yes & no.

Short-term

The switch in v0.2 was to let us start actually creating these datasets before all implementations start supporting zarr-developers/zarr-python#715, i.e. ome-zarr-* implementations for v0.2 can "blindly" assert the use of "/" as the dimension separator. Python implementations should be in the (enviable) position that it doesn't matter. If you put the metadata in place, it should work for either assuming your zarr-python version is >=2.8.0 (note: ongoing bug fixes) Does that track with what you are seeing or am I missing something on the xarray front?

Longer-term

Once all of this has propagated through the ecosystem I think this spec shouldn't really care about which array layout is used. So a v0.3 or later can drop the requirement all together. That, however, does not get you want you want in the general case with IPFS. Do you foresee it really being that bad? (I ask because I know that the flat directory is unusable at some scales.)

@joshmoore ah, thanks for the information! I was not aware of zarr-developers/zarr-python#715 .

On the xarray front, they are not tied to a specific zarr spec, as far as I know, so it should cause an issue. But, it seems that we should do the same here, i.e. both layouts, . or /, would be acceptable, as you suggest, correct?

I ask because I know that the flat directory is unusable at some scales.

I can see how this would be the case. In which store types do you experience this?

For IPFS, each entity is identified with a cryptographic-hash. Updating a chunk in a file means updating its identifier, but it also means updating the identifiers for all parent directories in the directory tree; so, deeply nested trees are not desirable.

both layouts, . or /, would be acceptable, as you suggest, correct?

That's the plan.

In which store types do you experience this?

DirectoryStore & FSStore

so, deeply nested trees are not desirable.

Understood. Thanks for the info.

What do you think is the best way to move forward here, @thewtex @joshmoore?
I would rather not change this now (especially since this change was done in a previous PR already and the changes here are just about syncing the different versions of the spec). Also, like @joshmoore said, not supporting hiearchical storage for the chunks can be a big issue for large datasets on the file-system.

So I would propose that we leave this as it is for now and eventually change the spec once most implementations are agnostic to the actual separator.

If really necessary and there are some implementations that require a flat hierarchy, this could then be added as an extra implementation specific requirement. (I.e. these implementations would not be able to read all ngff formats, but only a subset.)

Yes, I'd propose we leave 0.2 as hierarchical, focus 0.3 on axes, and then we can try to remove the 0.2 restriction ASAP.

constantinpape · 2021-05-17T10:42:14Z

Sorry, it took me a while to catch up with this. Holidays and then a fairly busy week otherwise.
I have left a comment for the hierarchical chunk storage.

@thewtex

In the interest of compatibility with xarray, we could update the way the axes are encoded with:
        "0/example/.zattrs": {
            "_ARRAY_DIMENSIONS": [
                "t",
                "c",
                "z",
                "y",
                "x", 
           ],

I am not so sure that this will work for the OME NGFF format. Potentially there could be multiple datasets which have different axes stored in a single zarr container.
How is this case handled by xarray right now?

Edit: Sorry, I did not think the above comment fully through. After looking at your example again, I realized that we can easily support this if we just require each "scale array" to contain the _ARRAY_DIMENSIONS field, which should be identical with multiscales:axes.
I will add this to this PR.

latest/index.bs

constantinpape · 2021-05-17T17:06:58Z

@joshmoore I think this is good to be merged, but I would suggest we make a 0.2.0 release in the main repository before that.

imagesc-bot · 2021-05-18T20:16:15Z

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

https://forum.image.sc/t/ngff-status-update-may-2021/52918/1

evamaxfield · 2021-05-19T00:12:10Z

latest/index.bs

    │
    ├── 0                     # Each multiscale level is stored as a separate Zarr array,
    │   ...                   # which is a folder containing chunk files which compose the array.
    ├── n                     # The name of the array is arbitrary with the ordering defined by
    │   │                     # by the "multiscales" metadata, but is often a sequence starting at 0.
    │   │
-    │   ├── .zarray           # All image arrays are 5-dimensional
+    │   ├── .zarray           # All image arrays must be up to 5-dimensional


@joshmoore shouldn't this be:

must be 5+ dimensional? OME-Tiff has the 8D / n-D spec and it would be a shame to lose that 😉

The goal of this PR is to allow for less(!) than 5D, so that it's not necessary to introduce singleton dimensions for 2D/3D/4D data.
There is an ongoing discussion about allowing for more than 5D, see #35. But that is out of scope for the current PR.

evamaxfield · 2021-05-19T00:13:38Z

latest/index.bs

+            "axes": [
+                "t", "c", "z", "y", "x"
+            ],


If we do support n-dimensional, (OME 8D spec / OME n-D spec) then this simply expands to include the dimension shorthands.

joshmoore · 2021-07-14T08:45:26Z

@constantinpape, a few points before I disappear on holidays:

Various 0.2/latest updates #50 will conflict but hopefully get us into a better place.
in general, 👍 for the strategy of this PR to make changes to latest/
we need a process and/or script for migrating latest's index.bs to a given version location
you'll need to change https://github.com/ome/ngff/blob/main/.github/workflows/publish.yml#L18 to uses: joshmoore/spec-prod@0_3 until someone with better JS skills than me makes that repo behave (I haven't checked if the upstream version does what we want yet)

joshmoore

Few last minute items to go on top of #50

joshmoore · 2021-07-14T09:19:35Z

latest/index.bs

@@ -2,8 +2,7 @@
 Title: Next-generation file formats (NGFF)
 Shortname: ome-ngff
 Level: 1
-Status: LS-COMMIT
-Status: w3c/ED
+Status: w3c/CG-FINAL


you likely want to revert these top-matter items

joshmoore · 2021-07-14T09:19:56Z

latest/index.bs

@@ -18,12 +17,12 @@ Editor: Sébastien Besson, Open Microscopy Environment (OME) https://www.openmic
 Abstract: This document contains next-generation file format (NGFF)
 Abstract: specifications for storing bioimaging data in the cloud.
 Abstract: All specifications are submitted to the https://image.sc community for review.
-Status Text: The current released version of this specification is
-Status Text: <a href="../0.1/index.html">0.1</a>. Migration scripts


This text should stay as is but point to the 0.3 that you will create when latest/index.bs is done.

joshmoore · 2021-07-14T09:20:49Z

latest/index.bs

@@ -551,6 +597,11 @@ Version History {#history}
      <td>Description</td>
    </tr>
  </thead>
+  <tr>


Plus add in your own changelog.

imagesc-bot · 2021-07-20T08:10:52Z

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

https://forum.image.sc/t/ome-zarr-napari/55435/1

joshmoore · 2021-08-24T10:41:39Z

I've dealt with merge conflicts and handled the "publication steps" (such as they are) if anyone would like to take a look.

joshmoore · 2021-08-24T10:42:30Z

If no objections, I'll merge this and move forward with https://github.com/orgs/ome/projects/17?add_cards_query=is%3Aopen

will-moore · 2021-08-24T10:59:23Z

Sorry, just a thought....

Do we want limit the last two axes to be ["y", "x"] (at least 2D)?
As I understand it, the idea of v0.3 was to allow less than 5D, but as it stands, these are all valid:

["c"],
["x", "y"],  # inverted
["c", "y", "t", "x"],

But do we want all clients that support the spec to also support all of those?

will-moore · 2021-08-24T11:06:27Z

Ah - sorry, I missed the existing caveat # with dimension order (t, c, z, y, x). which hasn't been changed by this PR.
It's just that this isn't mentioned in the list of axes rules in the text.
And there's still no 2D minimum - but maybe that's OK since we're all dealing with Images.
So I don't think this is a blocker.
Apologies for the noise!

sbesson

This new version of the specification has already been tested in various read and write scenarios across different implementations including omero-cli-zarr, vizarr, MoBIE. The specification changes look good so I am in favor of getting this published and completing the various tasks associated with OME-NGFF 0.3.

@joshmoore

Add axes field to multiscale metadata SHA: 2986bbe Reason: push, by @joshmoore Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

jni · 2021-08-29T10:44:39Z

Just spinning back up after my hiking-trip-turned-lockdown-staycation and I'm so delighted to find this merged! 🎉 Thank you SO much to everyone for your patience and perseverance with this! ❤️‍🔥 (◀️ just discovered that emoji and it's perfect!)

constantinpape added 2 commits April 26, 2021 11:08

Update latest to 0.2

bba6940

Add axes field to multiscales metadata

6b8a868

constantinpape marked this pull request as draft April 26, 2021 09:15

joshmoore reviewed Apr 26, 2021

View reviewed changes

latest/index.bs Outdated Show resolved Hide resolved

latest/index.bs Outdated Show resolved Hide resolved

latest/index.bs Show resolved Hide resolved

latest/index.bs Outdated Show resolved Hide resolved

constantinpape mentioned this pull request Apr 26, 2021

Spec development process #45

Open

constantinpape added 2 commits April 26, 2021 11:52

Merge remote-tracking branch 'ome/main' into spec-v03

a345247

Fix version numbers

9c57396

constantinpape marked this pull request as ready for review April 26, 2021 09:54

constantinpape mentioned this pull request Apr 26, 2021

Decoupling version numbers of different specifications #47

Closed

This was referenced Apr 27, 2021

Lift 5d requirement for images and move multiscales description into spec #39

Closed

Data dimensionality and axes metadata #35

Closed

joshmoore mentioned this pull request May 5, 2021

Compatibility with xarray #48

Open

Allow for less than 5d

e60ec27

thewtex reviewed May 6, 2021

View reviewed changes

joshmoore mentioned this pull request May 6, 2021

NCZarr - Netcdf Support for Zarr zarr-developers/zarr-specs#41

Open

Add _ARRAY_DIMENSIONS requirement for compatibility with xarray

cf76570

constantinpape commented May 17, 2021

View reviewed changes

latest/index.bs Outdated Show resolved Hide resolved

Minor formulation tweak

af7b947

evamaxfield reviewed May 19, 2021

View reviewed changes

This was referenced May 19, 2021

Add support for different file formats to spec mobie/mobie.github.io#49

Closed

Support for multi-channel labels #19

Open

constantinpape mentioned this pull request Jun 10, 2021

Napari plugin for ome-zarr #49

Closed

joshmoore mentioned this pull request Jul 12, 2021

v0.3 support hms-dbmi/vizarr#103

Closed

joshmoore requested changes Jul 14, 2021

View reviewed changes

joshmoore and others added 4 commits August 24, 2021 12:33

Store the current latest as 0.3

c46d9d5

Revert some latest changes and add changelog

47fa3dc

Merge branch 'main' into spec-v03

3e9908c

Add Constantin as editor

a981f98

joshmoore self-assigned this Aug 24, 2021

joshmoore requested a review from sbesson August 24, 2021 10:42

sbesson approved these changes Aug 24, 2021

View reviewed changes

joshmoore merged commit 2986bbe into ome:main Aug 24, 2021

bogovicj mentioned this pull request Feb 2, 2022

Proposing spaces and transforms #94

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add axes field to multiscale metadata #46

Add axes field to multiscale metadata #46

constantinpape commented Apr 26, 2021

joshmoore left a comment

constantinpape commented Apr 26, 2021

constantinpape commented May 5, 2021

thewtex left a comment

thewtex May 6, 2021

joshmoore May 6, 2021

thewtex May 7, 2021

joshmoore May 7, 2021

constantinpape May 17, 2021

joshmoore May 17, 2021

thewtex May 17, 2021

constantinpape commented May 17, 2021 •

edited

Loading

constantinpape commented May 17, 2021

imagesc-bot commented May 18, 2021

evamaxfield May 19, 2021

constantinpape May 19, 2021

evamaxfield May 19, 2021

joshmoore commented Jul 14, 2021

joshmoore left a comment

joshmoore Jul 14, 2021

joshmoore Jul 14, 2021

joshmoore Jul 14, 2021

imagesc-bot commented Jul 20, 2021

joshmoore commented Aug 24, 2021

joshmoore commented Aug 24, 2021

will-moore commented Aug 24, 2021

will-moore commented Aug 24, 2021

sbesson left a comment

jni commented Aug 29, 2021

Add axes field to multiscale metadata #46

Add axes field to multiscale metadata #46

Conversation

constantinpape commented Apr 26, 2021

joshmoore left a comment

Choose a reason for hiding this comment

constantinpape commented Apr 26, 2021

constantinpape commented May 5, 2021

thewtex left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Short-term

Longer-term

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

constantinpape commented May 17, 2021 • edited Loading

constantinpape commented May 17, 2021

imagesc-bot commented May 18, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshmoore commented Jul 14, 2021

joshmoore left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imagesc-bot commented Jul 20, 2021

joshmoore commented Aug 24, 2021

joshmoore commented Aug 24, 2021

will-moore commented Aug 24, 2021

will-moore commented Aug 24, 2021

sbesson left a comment

Choose a reason for hiding this comment

jni commented Aug 29, 2021

constantinpape commented May 17, 2021 •

edited

Loading