-
Notifications
You must be signed in to change notification settings - Fork 51
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
Add JSON schema #1426
base: dev
Are you sure you want to change the base?
Add JSON schema #1426
Conversation
46fcfcd
to
78fc678
Compare
638c575
to
4f4be4b
Compare
199a223
to
5b09f81
Compare
b6b057b
to
e99b0fb
Compare
3fb1ea9
to
e866604
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, thank you!
Wondering a bit if validation of an XML backend would be easier.... not sure.
# We need to exclude the thetaMode example since that has a different | ||
# meshesPath and the JSON schema needs to hardcode that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we able to patch this in check.py
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not very easily. The JSON schema is on the file system and the single .json files refer to each other by their file names. Changing this would require (1) traversing the entire JSON schema and overriding the meshes path, the particles path and the references and (2) somehow setting up python-jsonschema to cross-reference in-memory schemas which I don't even know if it supports that, both at runtime of check.py
.
The major issue of the JSON schema approach is that it is not a predictive parser (https://en.wikipedia.org/wiki/Recursive_descent_parser), meaning that it deals with If we want to seriously consider XML as an alternative, my opinion is that it must at least solve this particular issue .. which I doubt it can properly, since this involves a bit of parser theory and grammar transformation. This job is better done by handwritten parsers, which the openPMD-api and the openPMD-validator are. However, if we add an XML backend at some point, an XML schema is definitely at least worth a consideration. |
7a5fee0
to
e66de14
Compare
c9fb0c1
to
4df3d58
Compare
4df3d58
to
54a2850
Compare
54a2850
to
2a36992
Compare
2a36992
to
43b7bea
Compare
43b7bea
to
785dae7
Compare
785dae7
to
321130a
Compare
686d5af
to
58497f1
Compare
bade114
to
09d8d96
Compare
Written as .toml files for ease of documentation, maintailability and readability.
Needed for "compiling" the schema to JSON Also add a Makefile to further simplify this
Workflow documented in README.md
The JSON schema verification package does not like that
Both of the form "data not found in places where data was expected"
Verify all JSON-openPMD files written by testing against the schema
09d8d96
to
e370ccc
Compare
TODO:
Maybe add support for abbreviated JSON representationsNot nowI have added a CLI tool
openpmd-convert_json_toml
, exposing internally existing functionality to convert between TOML and JSON. I think it's better to define and maintain the schema in terms of TOML (it can be commented and is more legible), and just compile it to JSON for verification.x-ref #1436