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.

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

Schemas v0.1 v0.2 #76

Merged
merged 18 commits into from
Dec 13, 2021
Merged

Schemas v0.1 v0.2 #76

merged 18 commits into from
Dec 13, 2021

Conversation

will-moore
Copy link
Member

@will-moore will-moore commented Dec 1, 2021

This adds schemas, tests and samples for JSON schema support of v0.1 and v0.2.

Updates all schemas to NOT require version, name etc, since these are only SHOULD in the spec (noticed this when validating some existing v0.1 files).

Also adds plate samples and plate.schema for the HCS spec (tests for 0.1 only, since this hasn't changed since).

This also adds a way of specifying the SHOULD rules within the schema, using a second schema with allOf to add restrictions to the image.schema (see below - thanks @sbesson).
This more strict schema can be used to validate without failing on the first Error, allowing it to be used for reporting multiple suggestions for rules that SHOULD be obeyed, but aren't actually invalid.

Added tests that check that all valid files (except image_complete.json) give at least one SHOULD warning.

{
  "allOf": [
    {
      "$ref": "http://localhost:8000/image.schema"
    },
    {
      "properties": {
        "multiscales": {
          "items": {
            "required": [
              "version", "metadata", "type"
            ]
          }
        }
      }
    }
  ]
}

@will-moore
Copy link
Member Author

That last commit avoids the need to run httpserver in the test, which will be nice for ome-zarr-py validate use-case where we want to ship the schemas with the library and not load them from a URL each time (or have to serve them from httpserver).
However, it does require that the code has some knowledge of the schema to update appropriate $ref:

schema['allOf'][0]['$ref'] = IMAGE_SCHEMA_KEY

@will-moore
Copy link
Member Author

The last commit uses a better approach of subclassing the RefResolver to always load local schemas instead of remote.

Copy link
Member

@joshmoore joshmoore left a comment

Choose a reason for hiding this comment

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

In general, love all the examples and the fact that this covers all current versions. One general naming consideration about the URLs since those are something we want to last. I haven't (yet) gone through each example and schema, but I don't think I'll get to that before Thursday. If it helps to get this in here so that ome-zarr-py etc. can build on top of it, no objections. We should just review before we make the URLs live. (I assume at this point they don't actually get built into the spec page)

@@ -2,7 +2,6 @@
"@type": "ngff:Image",
"multiscales": [
{
"version": "0.2",
Copy link
Member

Choose a reason for hiding this comment

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

This was dropped from v0.3?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed to "0.1"

@@ -1,6 +1,6 @@
{
"$schema": "https://json-schema.org/draft/2020-12/schema",
"$id": "http://localhost:8000/image.schema",
"$id": "https://ngff.openmicroscopy.org/0.3/schemas/json_schema/image.schema",
Copy link
Member

Choose a reason for hiding this comment

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

The "schema(s)" in this URL are a bit much....

Copy link
Member Author

Choose a reason for hiding this comment

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

That is just the path we currently have in this repo. Since we're only supporting json_schema now, we could drop that dir. But the paths are even more painful at ome/ome-zarr-py#142 where I wanted to maintain the same directory structure as here, simply replacing https://ngff.openmicroscopy.org/ with a single path in that repo (expecting that we'll package the schemas as part of the release). So the path in that repo then becomes schemas/0.3/schemas/json_schema/image.schema!
The problem with is that in the NGFF repo, the version is at the top level, so we have a schemas directory under each version, but it's the other way around on ome-zarr-py where I wanted all the schemas to be under a top-level schemas/.
Need to unify the approach between the repos. Do we create top-level version dirs on ome-zarr-py?

@will-moore
Copy link
Member Author

Once we've "published" a schema to a public URL that people can use for validation, can we make any future changes to that schema, and how do we version it?
E.g. if we make a change/fix and then an instance file becomes valid/invalid as a result??

@will-moore
Copy link
Member Author

From the discussion this morning a plan of action:

  • Move schemas to e.g. /0.3/schemas/image.schema (remove extra json_schema dir)
  • Make ngff pip installable
  • Include get_schema(version, strict=False) from ome-zarr-py validation PR
  • Update bikeshed job to also publish schemas - see https://github.com/joshmoore/spec-prod

I think only the first point should be done in this PR?

@joshmoore
Copy link
Member

Merging as discussed this morning. As mentioned, we can re-review the URLs before going public with them.

@joshmoore joshmoore merged commit a20dd71 into ome:main Dec 13, 2021
github-actions bot added a commit that referenced this pull request Dec 13, 2021
Schemas v0.1 v0.2

SHA: a20dd71
Reason: push, by @joshmoore

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@sbesson sbesson added this to the 0.4 milestone Feb 1, 2022
@sbesson sbesson added this to Done in OME-NGFF 0.4 Feb 6, 2022
@sbesson sbesson mentioned this pull request Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants