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.

Sign up for GitHub

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

Specify NGFF 0.6-dev in output data #849

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+21 −2
Open

Specify NGFF 0.6-dev in output data #849

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions src/spatialdata/_io/format.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
from __future__ import annotations

from collections.abc import Iterator
from typing import Any

import ome_zarr.format
import zarr
from anndata import AnnData
from ome_zarr.format import CurrentFormat
from ome_zarr.format import CurrentFormat, Format, FormatV01, FormatV02, FormatV03, FormatV04
from pandas.api.types import CategoricalDtype
from shapely import GeometryType

Expand Down Expand Up @@ -45,7 +47,24 @@ def _parse_version(group: zarr.Group, expect_attrs_key: bool) -> str | None:


class SpatialDataFormat(CurrentFormat):
pass
@property
def version(self) -> str:
return "0.6-dev"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @bogovicj in case defining a specific pre-release version (i.e. dev1 or beta1) would make detecting differences in these upcoming versions more reliable.

See also ongoing conversation around 0.5.2

Copy link
Member

@joshmoore joshmoore Jan 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, going back and looking at the diff output in the description, I'd suggest we not use 0.6.* unless this also implements the Zarr v3 work of 0.5.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good point. Any suggestion on the string to use? Maybe something more verbose like 0.4-dev-transformations?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or 0.4-dev-spatialdata .

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 👍 for starting with 0.4-
  • 👍 for having "spatialdata" or similar in it
  • you might think about whether or not you have a version field from SD itself
  • I defer to you if you want "dev" in it or not (I don't think the validator, e.g., has special behavior for that. @will-moore?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @joshmoore.

you might think about whether or not you have a version field from SD itself
The SpatialData is versioned here:

spatialdata/src/spatialdata/_io/format.py

Line 233 in a7e9e99

SpatialDataContainerFormats = {

@will-moore I think your "no" answers to "(I don't think the validator, e.g., has special behavior for that. @will-moore?)". Do you have any preference for the 0.4- string?

Copy link
Member

@joshmoore joshmoore Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it'll need a little patching to parse the "0.4" out of a longer string if it's going to validate those.

Except this version explicitly does not validate, so perhaps it's enough that it's just not the known "0.4"?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can handle 0.4-* anything is fine.

I tried looking at some spatialdata data in the validator with...

$ ome_zarr view mouse_liver_data.zarr

...looking at the /labels/segmentation_mask/ the validator is complaining that /multiscales/0/coordinateTransformations/0 doesn't have scales or translation objects.
Instead you have "input":{"axes":[...]}, "output": {"axes":[...]}, "type": "identity"} which is heading towards the CoordinateSystems RFC-5 https://ngff.openmicroscopy.org/rfc/5/index.html but not quite the same.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I will adjust the coordinate transformations metadata together with @melonora in ome-zarr-models-py.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you agree, I'd go then with 0.4-dev-spatialdata and merge this PR.



def format_implementations() -> Iterator[Format]:
"""Return an instance of each format implementation, newest to oldest."""
yield SpatialDataFormat()
yield FormatV04()
yield FormatV03()
yield FormatV02()
yield FormatV01()


# monkeypatch the ome_zarr.format module to include the SpatialDataFormat (we want to use the APIs from ome_zarr to
# read, but signal that the format we are using is a dev version of NGFF, since it builds on some open PR that are
# not released yet)
ome_zarr.format.format_implementations = format_implementations


class SpatialDataContainerFormatV01(SpatialDataFormat):
Expand Down
Loading