Skip to content
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.

Sign up for GitHub

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

Adjust handling of unknown properties #456

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
Open

Adjust handling of unknown properties #456

Changes from 11 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
9303373
Adjust handling of unknown properties for better future-proof behavior.
rartino Feb 20, 2023
8ed4ab9
Adjust grammar
rartino Feb 20, 2023
1657e4c
Handle forwards-compatibility also for response_field query parameter.
rartino Feb 27, 2023
093dda7
Merge branch 'develop' into future_compatibility_unknown_property_names
rartino Feb 27, 2023
1fc0df2
Further improve forwards-compatibility with future additions
rartino Feb 28, 2023
b91e617
Fix unknown property -> unrecognized property
rartino Feb 28, 2023
0deec60
Apply suggestions from review
rartino Feb 28, 2023
3ebf054
Fix incorrect example on futureproof unknown field
rartino Mar 20, 2023
301d71d
Fix warning field name
ml-evs May 11, 2023
5d0d021
Fix rst formatting from review
rartino May 16, 2023
e6695e8
Clarify statement about new fields and null based on review
rartino May 17, 2023
793ad00
Apply suggestions from review
rartino Jun 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 22 additions & 13 deletions optimade.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,20 +423,17 @@ OPTIONAL properties with an unknown value, if requested explicitly via the :quer
The interaction of properties with an unknown value with query filters is described in the section `Filtering on Properties with an unknown value`_.
In particular, filters with :filter-fragment:`IS UNKNOWN` and :filter-fragment:`IS KNOWN` can be used to match entries with values that are, or are not, unknown for some property, respectively.

Handling unknown property names
-------------------------------
Handling unrecognized property names
------------------------------------

When an implementation receives a request with a query filter that refers to an unknown property name it is handled differently depending on the database-specific prefix:
When an implementation receives a request with a query filter or other mechanism (e.g., :query-param:`response_fields`) that refers to a property name that is not recognized, it MUST NOT treat this as an error but rather MUST issue a warning with :field:`code` set to :field-val:`unrecognized_property`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to understand this better, does "unrecognized property" now mean unsupported OPTIMADE properties too? I recall that prior this PR "unknown" stood for properties with database-specific prefixes. So now for example an implementation can choose to "not recognize" structures' chemical_formula_anonymous?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this change is the most vital part of this PR to improve forwards compatibility in OPTIMADE.

The point is, when we add, e.g., spacegroup to the standard, and you send a query to two databases asking to include spacegroup among the response fields (or referencing it in the filter), you do not want an error; you want it treated as null.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, so the client will have to inspect the warnings to make sure that the empty return list means the database does not implement spacegroup, not that it does not contain any structures with the queried space group.

Queries MUST be evaluated with the property treated as unknown, meaning comparisons are evaluated as if the property has the value :val:`null`.
If the unrecognized property is requested to be included in the response, it MUST be included with :val:`null` value.

* If the property name has no database-specific prefix, or if it has the database-specific prefix that belongs to the implementation itself, the error :http-error:`400 Bad Request` MUST be returned with a message indicating the offending property name.

* If the property name has a database-specific prefix that does *not* belong to the implementation itself, it MUST NOT treat this as an error, but rather MUST evaluate the query with the property treated as unknown, i.e., comparisons are evaluated as if the property has the value :val:`null`.

* Furthermore, if the implementation does not recognize the prefix at all, it SHOULD return a warning that indicates that the property has been handled as unknown.

* On the other hand, if the prefix is recognized, i.e., as belonging to a known database provider, the implementation SHOULD NOT issue a warning but MAY issue diagnostic output with a note explaining how the request was handled.

The rationale for treating properties from other databases as unknown rather than triggering an error is for OPTIMADE to support queries using database-specific properties that can be sent to multiple databases.
The rationale for not triggering errors for unrecognized properties is to enable clients to perform queries that reference properties defined in future versions of the OPTIMADE standard or database-specific properties, and have them handled only by the databases that recognize them.
Database-specific properties (as well as properties defined in future versions of this standard) SHOULD be defined in such a way that returning them with the value :val:`null` and treating them as :val:`null` in comparisons is a reasonable behavior for providers that do not support the property.
If this is not possible, and if the endpoint defines a :property:`<endpoint>_features` property (e.g., :property:`structure_features` for the :endpoint:`structures` endpoint), the implementation MUST define a database-specific identifier and place it in this list for those entries where the distinction between implementations that do and do not support the property is important.
Filters can then be formulated as, e.g., :filter-fragment:`_exmpl_strange_property IS UNKNOWN and structure_features HAS "_exmpl_strangeness"` to suppress results from implementations that do not support :property:`_exmpl_strange_property`.
rartino marked this conversation as resolved.
Show resolved Hide resolved
rartino marked this conversation as resolved.
Show resolved Hide resolved

For example, the following query can be sent to API implementations `exmpl1` and `exmpl2` without generating any errors:

Expand Down Expand Up @@ -719,7 +716,14 @@ Warnings
Non-critical exceptional situations occurring in the implementation SHOULD be reported to the referrer as warnings.
Warnings MUST be expressed as a human-readable message, OPTIONALLY coupled with a warning code.

Warning codes starting with an alphanumeric character are reserved for general OPTIMADE error codes (currently, none are specified).
Warning codes starting with an alphanumeric character are reserved for general OPTIMADE error codes.
The following are defined:

- :field-val:`unrecognized_property`: the request refers to an unrecognized property in the query parameters :query-parameter:`filter`, :query_parameter:`response_fields`, or in some other way.
Implementations SHOULD indicate in the warning field :field:`detail` which properties are unrecognized.
For future compatibility, unrecognized properties are handled as having value :val:`null`.
For more information, see `Handling unrecognized property names`_.

For implementation-specific warnings, they MUST start with ``_`` and the database-provider-specific prefix of the implementation (see section `Database-Provider-Specific Namespace Prefixes`_).

API Endpoints
Expand Down Expand Up @@ -2296,6 +2300,9 @@ database-provider-specific properties
Similarly, an implementation MAY add keys with a database-provider-specific prefix to dictionary properties and their sub-dictionaries.
For example, the database-provider-specific property :property:`_exmpl_oxidation_state`, can be placed within the OPTIMADE property :property:`species`.

Database-specific properties SHOULD be defined in such a way that returning them with the value :val:`null` and treating them as :val:`null` in comparisons is a reasonable behavior for providers that do not support the property.
See `Handling unrecognized property names`_ for more information.

- **Type**: Decided by the API implementation.
MUST be one of the OPTIMADE `Data types`_.
- **Requirements/Conventions**:
Expand Down Expand Up @@ -2848,6 +2855,8 @@ structure\_features
- MUST be sorted alphabetically.
- If a special feature listed below is used, the list MUST contain the corresponding string.
- If a special feature listed below is not used, the list MUST NOT contain the corresponding string.
- Clients SHOULD NOT assume they can represent the structure correctly if the :field:`structure_features` field indicates a feature not supported by the client or contains any unrecognized items.
- Implementations that use database-specific properties that must be supported by the client to correctly interpret the structural information MUST define a string with a database-provider-specific prefix and place it in this list for those entries that would be incorrectly interpreted by ignoring the property.
ml-evs marked this conversation as resolved.
Show resolved Hide resolved
- **List of strings used to indicate special structure features**:

- :val:`disorder`: this flag MUST be present if any one entry in the `species`_ list has a :field:`chemical_symbols` list that is longer than 1 element.
Expand Down