-
Notifications
You must be signed in to change notification settings - Fork 37
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
Changes to property definitions #457
Changes to property definitions #457
Conversation
Co-authored-by: Antanas Vaitkus <[email protected]>
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.
Looks good to me, I will approve once @vaitkus's suggestions are accepted.
Co-authored-by: Antanas Vaitkus <[email protected]>
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.
Looks good, thanks.
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.
Thank you for the changes.
Some minor adjustments to our property definitions according to what has been discussed in issues:
x-optimade-property/property-uri
into simply$id
, which is the JSON Schema standard way of communicating the same thing, see Replace property definitions x-optimade-property/property-uri with just an outer "$id" key? #442.x-optimade-type
to specify the OPTIMADE data type. It was concluded in SMILES property #392, SMILES data type #436 that this is necessary if we do not want to make things very complicated when/if we want/need to support more complex data types.The definition of the
type
field has thus been completely rewritten to take into account the quite direct relationship tox-optimade-property
.(Note: I mentioned earlier that I would take this in #445, but upon realizing that I had to do rather significant edits here, I decided not to mix the two up.)
Edit: Oh, and this PR also fixes a bug in that the mandatory
x-optimade-property/property-format
was not included in the response example.