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

Extract specification snippets as standalone JSON files #98

Merged
merged 9 commits into from
Mar 18, 2022

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented Feb 8, 2022

See #8, this is a proof-of-concept extracting the multiscales example snippet and using the JSON schema testing infrastructure introduced in #92.

See http://api.csswg.org/bikeshed/?url=https://raw.githubusercontent.com/sbesson/ngff/json_snippets/0.4/index.bs#multiscale-md for the rendered version of this PR.

Incidentally, this allowed to flag several issues with the snippet which were fixed in 7a62d84.

This work raised a few issues for managing these snippets moving forward:

  • 7a62d84 removes the inline comments as there is no such markup in JSON. Should we keep these as an explanatory sentence before the example?
  • I placed the JSON example under the valid_strict category introduced in #92. Alternate suggestions for category or naming welcome.
  • the long lines in the coordinateTransformations someway raise the question of the formatting for these snippets. Unless someone has a strong feeling re indentation and style, I assume we can be kept ad-hoc for now with the goal to improve readability

@constantinpape
Copy link
Contributor

  • 7a62d84 removes the inline comments as there is no such markup in JSON. Should we keep these as an explanatory sentence before the example?

Could we store them as yaml and make use of the fact that it supports comments somehow? (just a thought, not sure how this plays with the infrastructure here)

  • the long lines in the coordinateTransformations someway raise the question of the formatting for these snippets. Unless someone has a strong feeling re indentation and style, I assume we can be kept ad-hoc for now with the goal to improve readability

I think "ad-hoc with the goal to improve readability" sounds good

@sbesson
Copy link
Member Author

sbesson commented Feb 9, 2022

Could we store them as yaml and make use of the fact that it supports comments somehow? (just a thought, not sure how this plays with the infrastructure here)

But even then, the YAML snippets would be reconverted as JSON to be rendered as .zattrs examples in the specification? Or would you envision showing the same examples using different formats?

@constantinpape
Copy link
Contributor

But even then, the YAML snippets would be reconverted as JSON to be rendered as .zattrs examples in the specification?

Is there a way to load them as plaintext instead? Or convert them to yaml for rendering as well? Yaml could be rendered exactly the same as json + comments, since it's a superset. (Not sure if this is feasible with the bikeshed infrastructure of course)

@joshmoore
Copy link
Member

I imagine we could add a build.sh (or Makefile!) which has a pipeline of yaml -> .json -> #include.

@sbesson
Copy link
Member Author

sbesson commented Mar 4, 2022

While adding a YML-> JSON build step is certainly doable, my two main worries are 1- the added complexity both in terms of developing/building the specification and 2- the fact these comments will still be lost as part of the translation and will disappear from the specification.

Trying to come up with another alternative, we could take advantage of the fact that the specification is meant to be extensible and store these as custom explanatory JSON keys e.g.:

                     "coordinateTransformations": [
                          {
                              "type": "scale",
                              "scale": [1.0, 1.0, 0.5, 0.5, 0.5],
                              "comment": "the voxel size for the first scale level (0.5 micrometer)"
                         }

Thoughts @constantinpape ?

@joshmoore
Copy link
Member

re "comment": see an extended discussion at w3c/json-ld-syntax#32 (comment)

@sbesson
Copy link
Member Author

sbesson commented Mar 10, 2022

Thanks @joshmoore, it's good to know that others have the same problem and that there are discussions to formalize this concept although it's clear from the thread and w3c/json-ld-syntax#32 (comment) that there is no native JSON-LD solution at this point.

I pushed additional commits that implement a version of the proposal above, using in that case description as a "magic" key, in the sense that it allows to store additional information but is neither defined by the specification nor disallowed - see http://api.csswg.org/bikeshed/?url=https://raw.githubusercontent.com/sbesson/ngff/json_snippets/0.4/index.bs#multiscale-md for the output.

@joshmoore
Copy link
Member

Though I definitely like the cleanness of the solution, I'd be very careful with any magic keys.

@sbesson
Copy link
Member Author

sbesson commented Mar 17, 2022

Additional searches did not allow me to identify a solution that would both produce valid JSON while respecting the concerns expressed in #98 (comment) regarding the usage of magic/private/reserved keys in the official specification.

Largely inspired by https://google.github.io/styleguide/jsoncstyleguide.xml, the last set of commits (fbf4aee, ae8839c and a7038b6) makes the following proposal:

  • comments are reinserted into the extracted JSON as previously (using // as the separator but we can keep # if that's preferred)
  • the Python tests are updated to strip the comment lines before running the validation against the JSON schemas
  • a new Document conventions section is added to the specification which uses the standard RFC 2119 blurb and also clarifies the explanation scope of the comments in the JSON snippets

With this change, there should be no loss of information compared to the current published specification. The outcome is rendered at:

http://api.csswg.org/bikeshed/?url=https://raw.githubusercontent.com/sbesson/ngff/json_snippets/0.4/index.bs#document-conventions
http://api.csswg.org/bikeshed/?url=https://raw.githubusercontent.com/sbesson/ngff/json_snippets/0.4/index.bs#multiscale-md

Copy link
Contributor

@constantinpape constantinpape left a comment

Choose a reason for hiding this comment

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

This solution looks good to me @sbesson. (I would only change SHOULD NOT to MUST NOT for the remark on json comments since any comments would result in invalid json)

0.4/index.bs Outdated Show resolved Hide resolved
0.4/index.bs Outdated Show resolved Hide resolved
@joshmoore
Copy link
Member

#98 (comment) comments are reinserted into the extracted JSON as previously (using // as the separator but we can keep # if that's preferred)

Much happier with this than with magic keys based on the stage that we are at. 👍

Co-authored-by: Josh Moore <[email protected]>
@sbesson
Copy link
Member Author

sbesson commented Mar 17, 2022

Pushed minor changes. I'll leave until tomorrow for people to comment but otherwise I propose to get this merged to update the 0.4 specification.
I'll follow-up with a PR against latest with the similar changes but also introducing examples and JSON schemas.

@sbesson sbesson merged commit 8dec691 into ome:main Mar 18, 2022
@sbesson sbesson deleted the json_snippets branch March 18, 2022 09:53
github-actions bot added a commit that referenced this pull request Mar 18, 2022
Extract specification snippets as standalone JSON files

SHA: 8dec691
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.

3 participants