-
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
Add per entry and per property meta data field. #463
Merged
merkys
merged 43 commits into
Materials-Consortia:develop
from
JPBergsma:JPBergsma_add_meta
Jun 16, 2023
+80
−1
Merged
Changes from 1 commit
Commits
Show all changes
43 commits
Select commit
Hold shift + click to select a range
4170937
First draft metadata field.
JPBergsma 92c4d92
updated example so the numbers make sense.
JPBergsma 6fae33d
Added that metadata fields can have their own metadata.
JPBergsma d343a86
Add specifically that meta suffix must not be used other than for met…
JPBergsma 54b632c
Moved database specific fields to a seperate section and renamed them…
JPBergsma 5d186d9
Apply suggestions from code review
JPBergsma cb63bf2
Added 'metadata_for' field.
JPBergsma d0a4e65
Revert "Moved database specific fields to a seperate section and rena…
JPBergsma 9e64a3a
Revert "Add specifically that meta suffix must not be used other than…
JPBergsma 498f1f5
Merge branch 'JPBergsma_add_meta' of https://github.com/JPBergsma/OPT…
JPBergsma 6cfe92f
Moved mention that suffix '_meta' should not be used for regular fiel…
JPBergsma 1414c55
Add extra single quotes around the '_meta' suffix.
JPBergsma 81f2f96
Added example of a property definition for a metadata field.
JPBergsma b69b0e8
Update optimade.rst
JPBergsma 478a572
Clarifined nested meta data fields a bit.
JPBergsma d3b7050
Merge branch 'JPBergsma_add_meta' of https://github.com/JPBergsma/OPT…
JPBergsma 8ed79b5
Placed meta_data_for field in the property definition.
JPBergsma f1e179c
Removed property definition metadata_for field as it is now part of t…
JPBergsma a60eb5c
Added comma to improve readability.
JPBergsma df40172
Added comma to improve readability.
JPBergsma d16e97c
Merge branch 'JPBergsma_add_meta' of https://github.com/JPBergsma/OPT…
JPBergsma 1e81988
Postponed decision on whether or not nested metadata fields are allowed
JPBergsma 6f56a17
Moved the metadata fields to the JSON API per entry meta field as sug…
JPBergsma 97a0435
Removed x-optimade-metadata-for from example property declaration as …
JPBergsma 00a7a08
Tried to make it clearer when a server SHOULD return the meta data.
JPBergsma 7099a69
removed extra '.'
JPBergsma 09fffea
Apply suggestions from review
rartino cceabb5
Fix where metadata properties appear based on discussion
rartino 45b98b8
Remove the repetition of info about the data->meta field
rartino b95c465
Provide an example that is less objectable for metadata
rartino 134a906
Fix examples as requested by workshop
rartino 078f174
Apply suggestions from code review
JPBergsma e15aaff
Apply suggestions from review
rartino 9c4417f
Fix JSON example spelling error
rartino b36c372
Update optimade.rst
JPBergsma cc42d8f
Apply suggestions from review
rartino 52ad0c0
Remove global per-entry metadata properties
rartino 62f987d
Remove ability for per-entry global metadata
rartino aaddc82
Apply suggestions from review
rartino 656e1fb
Merged upstream develop
rartino 4feb8e1
Update optimade.rst
merkys 6992038
Cleanup trailing whitespace.
merkys 6ffe550
Apply suggestions from review
rartino File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Fix examples as requested by workshop
commit 134a9065fd7cd433249b461496787a7f827069cf
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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.
@sauliusg @giovannipizzi Right, what did we end up saying about this name?
I was somewhat in favor of "property_metadata" since this is the metadata channel for metadata of OPTIMADE properties when implemented in the JSON response format. I suppose one can argue that OPTIMADE properties are implemented in the JSON response format as attributes (except for id, type...) and as such, naming the channel "attributes_metadata" also makes some sense.
Any thought on this? Do we care at all? I'm assuming @JPBergsma prefers "attributes_metadata" since he proposes the change.
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.
I prefer "property" as this is the term used in OPTIMADE specification. Term "attributes" is specific to JSON:API, thus I would suggest us not to tie with it. If in future alongside JSON:API we introduce XML representation with OPTIMADE terms, having both "properties" and "attributes" may add unnecessary confusion.
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.
Sorry, I did not realize this suggestion was already posted.
During the discussion in the coffee room, we decided that the name would be "attributes_metadata".
In one of the examples on the white boards for the ranged properties, the term "property_metadata" was used, which is probably where @rartino got it from. To me, "attributes_metadata" sounds more fitting for the JSON response, but I do not have a strong preference for the name. If the majority wants "property_metadata", that's also fine with me.