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.
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
Add per entry and per property meta data field. #463
Changes from 40 commits
4170937
92c4d92
6fae33d
d343a86
54b632c
5d186d9
cb63bf2
d0a4e65
9e64a3a
498f1f5
6cfe92f
1414c55
81f2f96
b69b0e8
478a572
d3b7050
8ed79b5
f1e179c
a60eb5c
df40172
d16e97c
1e81988
6f56a17
97a0435
00a7a08
7099a69
09fffea
cceabb5
45b98b8
b95c465
134a906
078f174
e15aaff
9c4417f
b36c372
cc42d8f
52ad0c0
62f987d
aaddc82
656e1fb
4feb8e1
6992038
6ffe550
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.