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
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.

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.

Copy link

Choose a reason for hiding this comment

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

I've opened a PR at ome/ome-ngff-validator#45 with the validator deployed for testing at https://deploy-preview-45--ome-ngff-validator.netlify.app/
Does that help with what you need?

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 Will, but as Josh said here:

Except this version explicitly does not validate, so perhaps it's enough that it's just not the known "0.4"?
Not passing the validation is expected because the coordinate transformation specification that we implemented was based on an earlier draft and our plan is to finalize it in ome-zarr-models-py.

In this PR I made a patch to ome-zarr-py to be able to read the tensors even with partially incorrect metadata. We indeed handle the read/write of metadata in spatialdata, and use ome-zarr-py to read the tensors.

What would be needed here is to agree on an appropriate version number, such as 0.4-dev-spatialdata, that doesn't imply that validation is passed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Please in this comment you can see the full context of why we want to fix the version string now and we defer passing the validation later: #805 (comment).



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