Skip to content

feat: check for "units" field#76

Open
lubianat wants to merge 3 commits into
ome:mainfrom
lubianat:main
Open

feat: check for "units" field#76
lubianat wants to merge 3 commits into
ome:mainfrom
lubianat:main

Conversation

@lubianat

@lubianat lubianat commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Why?

The "unit" field is optional, so it is not validated.

This checks if the "units" field is used, which is almost certainly a mistake.

Noted on IDR/ome-ngff-samples#36 in a sample. This may be happening silently to other data out there.

tested locally with

{
  "attributes": {
    "ome": {
      "version": "0.5",
      "multiscales": [
...
          "axes": [
            {
              "name": "c",
              "type": "channel"
            },
            {
              "name": "z",
              "type": "space"
            },
            {
              "name": "y",
              "type": "space"
            },
            {
              "name": "x",
              "type": "space",
              "units": "micrometer"
            }
          ]
        }
      ],
      ...
    }
  },
  "zarr_format": 3,
  "
grafik

and for

grafik

# Why?

The "unit" field is optional, so it is not validated.

This checks if the "units" field is used, which is almost certainly a mistake.

Noted on IDR/ome-ngff-samples#36 in a sample. This may be happening silently to other data out there.
@netlify

netlify Bot commented Jun 5, 2026

Copy link
Copy Markdown

Deploy Preview for ome-ngff-validator ready!

Name Link
🔨 Latest commit 20b81ad
🔍 Latest deploy log https://app.netlify.com/projects/ome-ngff-validator/deploys/6a286c1e12290b000971f690
😎 Deploy Preview https://deploy-preview-76--ome-ngff-validator.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Closes ome#52

Change unit checks ("SHOULD") to warnings.

@will-moore will-moore left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was thinking myself that this could be a useful check to add. 👍

However, I realised in checking v0.6dev4 data that we miss all the axes checks because it's only looking for multiscale.axes above at const { axes, datasets } = multiscale;

If there's no axes there, then we should check at least the first coordinateSystem axes. E.g. axes = multiscale.coordinateSystems?.[0]?.axes.

co-authored by Claude Fable 5, reviewed/edited line by line
@lubianat

lubianat commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

added the checks @will-moore ; for disclosure: I used the brand new Fable 5 model for support, but reviewed/edited line by line. It does look like it did a fine job.

I am still to get a 0.6dev4 dataset and try out

@will-moore

Copy link
Copy Markdown
Member

Code looks good.

You could consider adding an invalid example to this repo, to test the error display.
E.g. like https://ome.github.io/ome-ngff-validator/?source=https://raw.githubusercontent.com/ome/ome-ngff-validator/refs/heads/main/test_samples/invalid/invalid_version.zarr/

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

Successfully merging this pull request may close these issues.

2 participants