Update structure endpoint to pre-alpha 0.10 spec

[ltalirz]

for @ml-evs

[ltalirz]

@ml-evs you can now use the constrained list we've created (in utils)
(I will as well)

[ltalirz]

On second thought.... it might need some more work to pass in the type of elements we are validating

[ml-evs]

I've just added the new chemical formula definitions from the recently merged Materials-Consortia/OPTIMADE#100, but there are still several fields to go.

• @ltalirz could you elaborate on your previous comment? I see that the constrained list length doesn't work for me (I've reworked it slightly to avoid using sequence_like which I couldn't find any references to.
• I don't like we currently format the mutli-line string descriptions, but outdenting them causes the swagger UI to think they are code blocks. Swagger is also having some difficulty displaying List[blah] entries, which get folded and misformatted.

[ltalirz]

I see that the constrained list length doesn't work for me

Ok, that's odd - note there are two tests for it that should check at least the basic functionality.
However, as it is implemented, one actually cannot pass in a type for validating the content of each element, which is kind of crucial for the constrained list to be useful
If you'd like to try and make progress on this that would be super useful (at the moment I'm swamped with deadlines).

I don't like we currently format the mutli-line string descriptions, but outdenting them causes the swagger UI to think they are code blocks.

Ok, if it works like this I guess we can live with it at the moment.

[ml-evs]

Ok, that's odd - note there are two tests for it that should check at least the basic functionality.
However, as it is implemented, one actually cannot pass in a type for validating the content of each element, which is kind of crucial for the constrained list to be useful

Could you give an example of how you intend the constrained list to be defined in structures.py? The test wraps it in another class and doesn't seem to use ConstrainedList at all.

[ltalirz]

The test uses conlist which does use ConstrainedList internally:
https://github.com/Materials-Consortia/optimade-python-https://github.com/Materials-Consortia/optimade-python-tools/blob/7db14b2c4a7df28a42771f13f935eb33c8c26b5b/optimade/server/models/util.py#L63-L75

Still, so far the constrained list (and the test) only checks the length of the list with no way of validating its contents. I wanted to look into this but at the moment have too many deadlines - unfortunately I don't see an example in pydantic of a constrained "container"; it's just int/str/... so one has to think

Furthermore, this is only the python part: I just checked and the constraint actually is not represented in the openapi.json in any way.

So, if you like, I would suggest to proceed with this PR without using the conlist at all for the moment (keeping track of missing validations either in the python code or via issues).
So we can get this merged and start using it - validation can always be improved lateron.

[ltalirz]

wait - "conlist was released today" (also @dwinston )
pydantic/pydantic#604

Ok, so it looks like this is exactly what we need.
Unfortunately, the latest fastapi (0.29.1) is not yet compatible with pydantic 0.29 - I tried and get an error

oading test structures...
inserting test structures into collection...
done inserting test structures...
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/leopold/Personal/Postdoc-MARVEL/Projects/2019-06-11_14_Optimade/optimade-python-tools/optimade/server/main.py", line 87, in update_schema
    json.dump(app.openapi(), f, indent=2)
  File "/Users/leopold/Applications/miniconda3/envs/optimade/lib/python3.6/site-packages/fastapi/applications.py", line 79, in openapi
    openapi_prefix=self.openapi_prefix,
  File "/Users/leopold/Applications/miniconda3/envs/optimade/lib/python3.6/site-packages/fastapi/openapi/utils.py", line 241, in get_openapi
    flat_models = get_flat_models_from_routes(routes)
  File "/Users/leopold/Applications/miniconda3/envs/optimade/lib/python3.6/site-packages/fastapi/utils.py", line 31, in get_flat_models_from_routes
    body_fields_from_routes + responses_from_routes
TypeError: get_flat_models_from_fields() missing 1 required positional argument: 'known_models'

So let's proceed as described above, and make an issue to include these constraints lateron once a compatible version of fastapi is released.

[ml-evs]

Sure thing, didn't mean to drag you away from other work! I'm going to attack another chunk of the structure attributes now and will only request your review (and others) when it's done. Cheers @ltalirz!

[ml-evs]

I think I've got everything in the current development spec from structure now. Happy to wait for any new changes in v0.10, or merge now. Outstanding issues:

• Still no reliable way to constrain the length and type of a list, pending pydantic/fastapi updates.
  • We also need to handle constrained lists of constrained lists, e.g. lattice_vectors.
• Swagger renders the docs for List in a bit of a strange way, with the type orphaned after the description.
• It would be nice to be able to constrain properties based on other properties, e.g. cartesian_site_positions should have length nsites (and vice versa) etc, though I guess this is beyond the powers of the schema.

[ltalirz]

Thanks a lot @ml-evs !

I don't think we need to wait until 0.10 - if there are changes, they can be tackled then.
Just one minor request.

[ltalirz]

By the way, you'll need to approve as this is "my PR".
Once you merge, I suggest you open issues for the points you mention. I've added a label "schema" that we can use to collect such to-do items