-
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
Adding the property_ranges
query parameter and associated metadata
#481
base: develop
Are you sure you want to change the base?
Adding the property_ranges
query parameter and associated metadata
#481
Conversation
Thanks for writing this up! Why did you move the whole section "Transmission of large property values"? It makes it tricky to see what has changed. Can you move it back? Also, my interpretation of the workshop discussions was to make the |
It seemed more logical to place the metadata section before the "Transmission of large property values" section, as the "Transmission of large property values" section depends on the metadata section but not the other way round. So it is more useful to read the metadata section before the "Transmission of large property values" section. |
You are right, I could still separate the property ranges query parameter and the extra metadata fields from the transmission of large data section, as each could still be used separate from the other. I'll try to do this next week. |
optimade.rst
Outdated
@@ -567,6 +517,131 @@ Example of the corresponding metadata property definition contained in the field | |||
} | |||
// ... | |||
|
|||
|
|||
Transmission of large property values |
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.
To clarify why I moved this section:
The section "Transmission of large property values" uses features from the section "Metadata properties". But the section "Metadata properties" does not depend on the section "Transmission of large property values" so I therefore placed the "Transmission of large property values" after the "Metadata properties" section.
I have made two small changes in the "Transmission of large property values" section:
Line 534 I replaced "one of alternative partial data formats " to "one of the partial data formats".
As I do not think the word alternative adds anything here.
Line 537: I broke the sentence into two parts and replaced "from the response, with three different links" with "from the response and includes three different links"
Make sure each sentence starts on a new line. Co-authored-by: Antanas Vaitkus <antanas.vaitkus90@gmail.com>
d2a18b7
to
a84c72e
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.
Mostly formatting changes and suggesting rewordings -- I think I can go through and force most of them in if there are no objections
property_ranges
query parameter and associated metadata
This PR appears a bit stalled and is important. @JPBergsma do you plan to continue working on this, or are you ok with me (or someone else) starting to merge changes into it? |
This PR makes a number of references to "range_ids", which is not the term used in the property definition PR #445. This needs to be adjusted. |
Co-authored-by: Antanas Vaitkus <antanas.vaitkus90@gmail.com>
optimade.rst
Outdated
|
||
- :field:`range_ids`: List of Strings. | ||
A list with an identifier for each dimension of the property. | ||
The outermost dimension of a nested array should come first. |
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.
The outermost dimension of a nested array should come first. | |
The outermost dimension of a nested array MUST come first. |
I think making this a MUST would make it easier to use both for client and server.
optimade.rst
Outdated
- :field:`range_ids`: List of Strings. | ||
A list with an identifier for each dimension of the property. | ||
The outermost dimension of a nested array should come first. | ||
If, within one entry, dimensions for two or more properties share the same :field:`range_id` those dimensions should be thought of as the same dimension. |
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.
If, within one entry, dimensions for two or more properties share the same :field:`range_id` those dimensions should be thought of as the same dimension. | |
If, within one entry, dimensions for two or more properties share the same :field:`range_id` those dimensions should be treated as the same dimension. |
optimade.rst
Outdated
A list with an identifier for each dimension of the property. | ||
The outermost dimension of a nested array should come first. | ||
If, within one entry, dimensions for two or more properties share the same :field:`range_id` those dimensions should be thought of as the same dimension. | ||
For example, if both the :property:`energy` and :property:`cartesian_site_positions` of a molecular dynamics trajectory share a range_id of :val:`frames`. |
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.
For example, if both the :property:`energy` and :property:`cartesian_site_positions` of a molecular dynamics trajectory share a range_id of :val:`frames`. | |
For example, both the :property:`energy` and :property:`cartesian_site_positions` of a molecular dynamics trajectory may share a `:field:range_id` of :val:`frames`. |
Co-authored-by: Andrius Merkys <andrius.merkys@gmail.com>
This PR describes how a client can request a specific part of a property that is returned as partial data.
I have tried to keep things as much as possible the same as in the original ranged properties proposal #452.