Skip to content

image-label: JSON schemas and clarifications#130

Merged
sbesson merged 15 commits intoome:mainfrom
sbesson:image-label_spec
Jul 12, 2022
Merged

image-label: JSON schemas and clarifications#130
sbesson merged 15 commits intoome:mainfrom
sbesson:image-label_spec

Conversation

@sbesson
Copy link
Member

@sbesson sbesson commented Jun 14, 2022

Fixes #125

This PR reviews the image-label specification and brings it inline with recent improvements made to other specifications including:

  • the addition of JSON schemas, a minimal schema enforcing all MUST requirements and a strict schema enforcing all SHOULD requirements, together with tests
  • the extraction of the embedded JSON snippets as standalone examples and their correction to be compliant with the schema
  • an update to the specification paragraph to clarify the content and requirement levels of all keys defined by the specification

In addition to reviewing the validation tests keep passing and that the modified text is sensible, the two biggest changes that should be reviewed, notably by the original authors of the specification (all marked as reviewers of this PR) are:

  • the current specification defines colors and properties keys with no requirement level and does not define the version key. In addiition to clarifying the expected content, this PR proposes a set of requirement levels using the keywords from RFC2119 and implemented through the JSON schemas
  • the name of the JSON schema is initially set to strict_image-label.schema to follow the convention used for other JSON schemas while keeping the hyphen separator using in image-label. The mixture of underscore and hyphen is most likely confusing. Should we change to underscores?
    Update (2022-07-05): updated the proposal to use label.schema and strict_label.schema

@github-actions
Copy link
Contributor

Automated Review URLs

@joshmoore
Copy link
Member

Automated Review URLs

Argh. Again not working.

Otherwise, 👍 though I agree that the schema name is not particularly friendly.

@sbesson
Copy link
Member Author

sbesson commented Jun 23, 2022

Otherwise, 👍 though I agree that the schema name is not particularly friendly.

Open to alternative suggestions but my concern is that this might force us to modify the name of existing published schemas - https://github.com/ome/ngff/tree/main/0.4/schemas.

@will-moore
Copy link
Member

LGTM 👍

@sbesson
Copy link
Member Author

sbesson commented Jul 5, 2022

Re the remaining sticking point (the name), considering simply naming the schemas as label.schema and strict_label.schema.

@will-moore
Copy link
Member

Those names sound fine to me 👍

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.

@joshmoore
Copy link
Member

@sbesson : is it fair to say that this "fixes #125" as well?

@sbesson
Copy link
Member Author

sbesson commented Jul 11, 2022

Yes and furthermore it only fixes #125. The previous description was erroneous (now updated)

@sbesson sbesson merged commit 65289c6 into ome:main Jul 12, 2022
github-actions bot added a commit that referenced this pull request Jul 12, 2022
image-label: JSON schemas and clarifications

SHA: 65289c6
Reason: push, by @sbesson

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

image-label metadata: invalid JSON in example

4 participants