Add 0.4 schemas and examples #87

will-moore · 2022-01-28T14:18:12Z

Validates the updated axes and transformations lists, according to latest spec discussed in #85.
Adds a bunch of valid and invalid examples, tested by the image.schema.

Fixes #82

…tions

Mandatory scale

constantinpape · 2022-01-31T18:16:22Z

Thanks for working on this @will-moore. I will give this a closer look once we have merged #85.

sbesson

Pushed a few extra commits to this PR adding more invalid examples and their associated constraints expressed as JSON schemas. From my side, happy to get this merged in its current state but I'll leave @constantinpape to take a look.

Probably the next todo will be for the schema to handle the definition coordinateTransformations at different levels (dataset, multiscale object) as #85. As this conversation still needs to get finalized and to simplify the review, I'd propose to get this in a separate PR.

Also to avoid unnecessary copy-n-paste, I think it should be possible to use $ref and $defs to make some of these definitions re-usable at different levels - see https://json-schema.org/understanding-json-schema/structuring.html

constantinpape

A few comments from my side:

In the current version of v0.4 units and type only have a SHOULD for the expected values.
This brings up the general question: how do we deal with the SHOULD requirements? Maybe we need two schemas, one that is strict and fails for SHOULD and one that is lenient and allows it?
I think that axes and coordnateTransforms should get their own schema, as they are meant to be referenced in other places in the spec. (But it's not strictly necessary to do this now.)

(p.s. I just read the comment by @sbesson which brings up similar points about axes and coordinateTransforms)

constantinpape · 2022-02-02T09:50:08Z

0.4/examples/invalid/invalid_axis_type.json

+            "axes": [
+                {
+                    "name": "y",
+                    "type": "invalid",


This is currently a SHOULD, see https://github.com/ome/ngff/blob/main/latest/index.bs#L217. Not sure how exactly we want to tackle this. I will leave a more general comment once I went through everything.

Agreed, this should not fail validation but probably be flagged by the stricter schema. I propose to open a follow-up PR with the infrastructure to differentiate the two of them.

Actually, re-reading this example, this should remain invalid.

As you rightfully mentioned type is a SHOULD and that is not sufficient ground to fail the strict schema. However there is only one space axe within the multiscales dictionary

yes, you're right. But the name of this example is a bit misleading then.

And in fact it is failing because type is not space, channel or time rather than only-1-space-axis.
So the schema should allow 1 'invalid' axis.

And in fact it is failing because type is not space, channel or time rather than only-1-space-axis.
So the schema should allow 1 'invalid' axis.

I am not sure what you mean by this @will-moore. But I agree that this one should fail the strict spec because it has only 1 spatial axis, as @sbesson points out. But I would maybe rename the example to make that clear.

Nods, that's probably more tests worth injecting but my understanding of the type restrictions is the following for multiscales.axes:

1 space axes + anything else: invalid

2 (or 3) space axes, 1 channel and/or 1 time: valid

2 (or 3) space axes, 1 channel or 1 time), 1 custom type: valid but failing the strict schema

2 (or 3) space axes, 1 channel or 1 time, 1 unspecified: valid but failing the strict schema

2 (or 3) space axes, 2 customs or 2 unspecified or (1 custom and 1 unspecified): valid but failing the strict schema

4+ space axes + anything else: invalid

2 (or 3) space axes, 1 channel or 1 time, 1 unspecified: valid but failing the strict schema

with 3 spatial axes this would fail because we would have > 5 dim. Same for some of the other examples you bring up.
But otherwise I agree with your list.

Heh the or in between 1 channel and 1 time was meant to capture that but I certainly failed to write the different combinations in a concise way. Expanding a bit, all the following set of axes types should be valid according to the minimal schema but should fail the strict schema:

2 space, 1 channel, 1 unspecified

2 space, 1 time, 1 unspecified

3 space, 1 channel, 1 unspecified

3 space, 1 time, 1 unspecified

constantinpape · 2022-02-02T09:50:47Z

0.4/examples/invalid/invalid_axis_units.json

+                {
+                    "name": "y",
+                    "type": "space",
+                    "units": "micron"


Same as for invalid type: this is currenrtly a SHOULD: https://github.com/ome/ngff/blob/main/latest/index.bs#L218

constantinpape · 2022-02-02T09:52:21Z

0.4/examples/invalid/mismatch_axes_units.json

+                {
+                    "name": "t",
+                    "type": "time",
+                    "units": "micrometer"


Similar to the comment before this is technically valid if we have a SHOULD in the units.

Agreed, although as discussed yesterday, this is typically a real-world example where I find the consequences of the SHOULD requirement nonsensical. I'll open a corresponding issue

Agreed, although as discussed yesterday, this is typically a real-world example where I find the consequences of the SHOULD requirement nonsensical.

Yeah, I agree that the consequences here are weird, but probably better to open an issue and not block the current release.

(and at least this would fail if we had a strict schema)

constantinpape · 2022-02-02T09:55:34Z

0.4/schemas/image.schema

+                "path": {
+                  "type": "string"
+                },
+                "coordinateTransformations": {


I think that coordinate transformations should be moved to a separate schema.

Agreed, separating axes and coordinateTransformations matches the way the specification is written and should increase the readability and the management of these schemas.

A technical side, as these schemas are not yet published (and we need to have some deeper discussions around URL as well as packaging - see #77), I suspect handling separate JSON schema files will require to make some decisions as relative references are not resolvable. Since these concepts are only used within the context of multiscales in v0.4, my inclination would be to start by defining internal sub-schemas using #defs - see #87 (review). As the intent is to use these concepts in other portions in the specification, I propose we review this assumption and define axes.schema and coordinateTransformations.schema when this becomes a requirement.

I agree, for now using them to #defs in the same schema is probably the best solution.

constantinpape · 2022-02-02T09:56:09Z

0.4/schemas/image.schema

+              "0.4"
+            ]
+          },
+          "axes": {


Axes should also be moved to a separate schema.

constantinpape · 2022-02-02T10:24:49Z

Ok, after the feedback from @sbesson I suggest we proceed as follows:

In this PR:
- split examples/invalid into examples/invalid_strict and examples/invalid_loose (or so; I don't mind changing the names, but there should be two folders ;)). And move the examples in the corresponding folder.
- make sure that all valid examples and invalid_loose pass the schema; invalid_strict fail the schema
- move "axes" and "coordinateTransformations" to #defs to improve readability
follow up work:
- introduce separate schemas for MUST and SHOULD, make sure that they validate the corresponding examples correctly
- move "coordinateTransformations" and "axes" to their own schemas

constantinpape

I guess the separation of examples and following SHOULD / MUST schemas is a bit more tricky. So this is good to be merged from my side (and continued work could be done after the v0.4 release.)

will-moore · 2022-02-02T12:38:41Z

@constantinpape @sbesson I think that's the essential things done for this PR.

Question: when coordinateTransformation is used at the multiscales level, MUST it include a scale object (as when used at the dataset level)? Currently the schema enforces that, since the same coordinateTransformations ref is used in both places.

constantinpape · 2022-02-02T12:51:24Z

Question: when coordinateTransformation is used at the multiscales level, MUST it include a scale object (as when used at the dataset level)? Currently the schema enforces that, since the same coordinateTransformations ref is used in both places.

@will-moore Yes, this is consistent with what we write in the spec right now, so I would go ahead and merge this.
Longer term: as far as I recall it's possible to extend definitions in jsonschema with a bit of magic, so that we don't need to require this in the coordinateTransformation schema directly.

@sbesson

Add 0.4 schemas and examples SHA: 430a9ae Reason: push, by @sbesson Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

@sbesson

Add 0.4 schemas and examples SHA: 430a9ae Reason: push, by @sbesson Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

will-moore added 3 commits January 28, 2022 14:17

Add 0.4 schemas and examples

f07a613

ensure 'time' and 'space' axes have valid units

1f584b0

Require 'scale' or 'translation' transformation(s)

78b1698

will-moore marked this pull request as ready for review January 28, 2022 16:38

Add new-lines to end of json examples

e92b6ef

will-moore mentioned this pull request Jan 28, 2022

Finalize axes & initial transformation #85

Merged

will-moore and others added 4 commits January 31, 2022 11:24

Rename to coordinateTransforms

a434669

Add example of dataset with missing scale element

eb2d997

Add containment requirement for scale element in coordinateTransforma…

b993d8b

…tions

Merge pull request #1 from sbesson/scale_mandatory

df897f1

Mandatory scale

sbesson added 5 commits January 31, 2022 21:33

Add additional examples of valid/invalid axes

456afea

Add requirement for 2/3 space axes via contains

6199e5c

Handle custom axes

cecafa8

Set axes maxItems to 5

ac87d92

Add maxContains to coordinateTransformations to enforce only one scale

8a165d7

sbesson approved these changes Feb 1, 2022

View reviewed changes

sbesson requested a review from constantinpape February 1, 2022 12:27

sbesson modified the milestone: 0.4 Feb 1, 2022

constantinpape mentioned this pull request Feb 2, 2022

Update data creation for v0.4 ome/ome-ngff-prototypes#2

Merged

constantinpape reviewed Feb 2, 2022

View reviewed changes

will-moore added 2 commits February 2, 2022 11:44

Move coordinateTransformations to

bd1618c

Move axes into

3067dd9

constantinpape approved these changes Feb 2, 2022

View reviewed changes

sbesson mentioned this pull request Feb 2, 2022

Disallow mismatching axes units and types #91

Open

will-moore added 3 commits February 2, 2022 12:05

duplicate_axes_name.json not invalid with schema

40a15df

Remove strict requirement for valid axis units

95c20ed

Add uniqueItems to axes

36340be

validate multiscales:coordinateTransformations

2459b15

sbesson merged commit 430a9ae into ome:main Feb 2, 2022

sbesson mentioned this pull request Feb 2, 2022

0.4 schemas: add testing infrastructure for strict and base validation #92

Merged

This was referenced May 9, 2022

bioformats2raw.layout #112

Merged

Proposal: consolidate JSON schema tests using keys #116

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add 0.4 schemas and examples #87

Add 0.4 schemas and examples #87

will-moore commented Jan 28, 2022 •

edited

Loading

constantinpape commented Jan 31, 2022

sbesson left a comment

constantinpape left a comment •

edited

Loading

constantinpape Feb 2, 2022

sbesson Feb 2, 2022

sbesson Feb 2, 2022

constantinpape Feb 2, 2022

will-moore Feb 2, 2022

constantinpape Feb 2, 2022

sbesson Feb 2, 2022

constantinpape Feb 2, 2022

sbesson Feb 2, 2022 •

edited

Loading

constantinpape Feb 2, 2022

constantinpape Feb 2, 2022

sbesson Feb 2, 2022

constantinpape Feb 2, 2022

constantinpape Feb 2, 2022

constantinpape Feb 2, 2022

sbesson Feb 2, 2022

constantinpape Feb 2, 2022

constantinpape Feb 2, 2022

constantinpape commented Feb 2, 2022 •

edited

Loading

constantinpape left a comment

will-moore commented Feb 2, 2022

constantinpape commented Feb 2, 2022

Add 0.4 schemas and examples #87

Add 0.4 schemas and examples #87

Conversation

will-moore commented Jan 28, 2022 • edited Loading

constantinpape commented Jan 31, 2022

sbesson left a comment

Choose a reason for hiding this comment

constantinpape left a comment • edited Loading

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

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

sbesson Feb 2, 2022 • edited Loading

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

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 Feb 2, 2022 • edited Loading

constantinpape left a comment

Choose a reason for hiding this comment

will-moore commented Feb 2, 2022

constantinpape commented Feb 2, 2022

will-moore commented Jan 28, 2022 •

edited

Loading

constantinpape left a comment •

edited

Loading

sbesson Feb 2, 2022 •

edited

Loading

constantinpape commented Feb 2, 2022 •

edited

Loading