Review the required properties of StructureResourceAttributes in openapi.json

[merkys]

As of now, required properties of StructureResourceAttributes do not reflect the OPTiMaDe specification very well, as most of the properties can be excluded using response_fields.

[CasperWA]

In terms of pydantics "required" and OPTiMaDe's "required in response", I completely agree. However, in the server it is handled correctly. But that doesn't change the fact that the OpenAPI is showing something misleading, for sure.

[merkys]

Right. I assume the JSON schema from the OpenAPI could be used to validate the responses, and sometimes they will contain none of the excludable properties.

[ml-evs]

I think this is the appropriate issue to restart this discussion...

There is still an outstanding question about how we should handle these fields in pydantic and in OpenAPI. Are "SHOULD" fields "required"? It is impossible to validate our StructureResource automatically with pydantic with some fields missing, unless we made them Optional and added conditionals to the pydantic validators.

We might be able to get around this in the implementation validator by just ignoring documents that are not "full" of all the "SHOULD" fields (i.e. not returning an error), but then obviously their contents will not be well-checked.

Originally posted by @ml-evs in #482

Things we might consider doing (not mutually exclusive, but I think we need to do at least one...)

1. Change our models to use Optional almost everywhere, so that the OpenAPI spec has very few required fields. We would then need to figure out how to validate these models effectively in pydantic.
2. Add an OpenAPI extension "support level" key (e.g. x-optimade-support-level, see SHOULD/MUST/OPTIONAL fields in models #453) to the OpenAPI and document it in the specification, stating that fields with x-optimade-support-level=SHOULD are allowed to be null/missing.

I guess it boils down to whether we want a spec that follows what people "SHOULD" do, with exceptions as special cases that we explicitly allow (which is my personal [biased] feeling for the spirit of the specification), or whether we should have a specification that allows almost everything.

[CasperWA]

Things we might consider doing (not mutually exclusive, but I think we need to do at least one...)

1. Change our models to use `Optional` almost everywhere, so that the OpenAPI spec has very few required fields. We would then need to figure out how to validate these models effectively in pydantic.

This seems like the most straight-forward solution, one that should definitely be put in place.
We can then validate according to the inter-field requirements, failing an OPTIMADE structures validation, e.g., if cartesian_site_positions is present, but species_at_sites is not, since they depend on each other's presence.

2. Add an OpenAPI extension "support level" key (e.g. `x-optimade-support-level`, see #453) to the OpenAPI and document it in the specification, stating that fields with `x-optimade-support-level=SHOULD` are allowed to be null/missing.

I guess it boils down to whether we want a spec that follows what people "SHOULD" do, with exceptions as special cases that we explicitly allow (which is my personal [biased] feeling for the spirit of the specification), or whether we should have a specification that allows almost everything.

I am not sure exactly what you have in mind for the boundaries here, but at least for the pydantic models, the whole point is self-validation and making sure we can use the data structures presented in the specification in practice. So the boundaries should be such that both of these points are satisfied (validation + practical usage).

In the end, I think it would be prudent to add awareness in the pydantic models about what attributes are SHOULD/MUST etc. However, how do we encode this information in the models, and how do we retrieve and use it subsequently, e.g., for the validator?

[merkys]

1. Change our models to use Optional almost everywhere, so that the OpenAPI spec has very few required fields. We would then need to figure out how to validate these models effectively in pydantic.

This sounds like the most natural solution to me.

[CasperWA]

This is currently being (partly) addressed by @ml-evs in #453

[ml-evs]

This was closed in #453 and #560 by using pydantic's required but nullable fields.