The optimade-python-tools from materials-consortia

Update models with new levels of REQUIRED response properties

With the merge of Materials-Consortia/OPTIMADE#210 new levels of what is OPTIONAL and what is REQUIRED for properties have been introduced.
Our schema models (for updating to v0.10.1) should be updated accordingly.

Django vulnerability

According to GItHub django >=2.2.0 and <2.2.8 is vulnerable.

Package structure

All of the tests can be stored in a separate module (folder) to reduce clutter ins the package code. inspiration or like fastapi etc.
from the root folder, the images can be moved into an images folder and relative path can be used in the markdown.
in the future, maybe somebody will have time to write proper documentation for this package :)

optimade-python-tools
| - docs
| - images
| - optimade
    | - filterparser
        | -  grammars
    | - filtertransformers
    | - model
    | - server
| - tests
    | - filterparser
    | - filtertransformers
    | - model
    | - server

Lark grammar file for v0.9.8

Based on the latest Filter Language EBNF Grammar (v0.9.8-develop), we need a corresponding optimade/grammar/v0.9.8.g file that translates the raw EBNF to lark-flavored form.

@waychal you were working on this last week. Can you please start a work-in-progress pull request for this ASAP? I don't think the filter grammar will change significantly for 1.0.

Possibly make /info/{endpoint} dynamic

It may be possible to make /info/{endpoint} dynamic in the server.
This is not crucial, but would serve as a good example of how to implement it in one's own OPTiMaDe server implementation.

NOT filter operation of mongo query for complex expressions

This implementation of NOT operation fails in the case when it has been applied for a "complex" expression. Here are some example for the inputs and outputs of a Transformer:

"NOT a>1" -> {'a': {'$not': {'$gt': 1}}}
"NOT a>1 AND b>1" -> {'$and': [{'a': {'$not': {'$gt': 1}}}, {'b': {'$gt': 1}}]}
"NOT (a>1 AND b>1)" -> {'$and': {'$not': [{'a': {'$gt': 1}}, {'b': {'$gt': 1}}]}}

In the case of the last one the transformer gives you a rubish query.

switch license to MIT

come up with suggested toolchain for validating existing optimade API against openapi.json

One could of course use the python tools available here (or e.g. openapi-core) but I'm sure people also would like to be able to test directly against the openapi.json file.

For tools that do this, see "Data Validators" and "Testing" here: https://openapi.tools/

add JSON schema API

See Materials-Consortia/API#18

info endpoint duplicated?

Looking at the autodoc generated by swagger, it somehow seems like the info endpoint is duplicated

https://petstore.swagger.io/?url=https://raw.githubusercontent.com/Materials-Consortia/optimade-python-tools/master/openapi/openapi.json#/

there should be no info/{entry} correct?
under the "Info" header, optimade/v0.10.0/info and optimade/v0.10.0/info/{entry} appears twice
optimade/info also appears under "Structure", reference, etc.

Add missing /references endpoint to server

/references is a new addition to the spec (v0.10.0) and should be added to the minimal server here, along with some test data.

Constraining list/array types in the schema

I don't have time for a full explanation (perhaps @ltalirz/@dwinston can elaborate), but we still have issues in constraining e.g. the lengths of lists in a way that is compatible with both pydantic and FastAPI.

Currently we have conlist which has just been released in pydantic, but has no effect on the generated API pydantic/pydantic/issues/604. It is not clear whether the types inside the conlist can be constrained, as of yet.

We also need a way to constrain lists of lists, e.g. ensuring lattice vectors are always 3x3, and if possible, constraining e.g. cartesian_site_positions to have length nsites, in order to fully validate a JSON response against the spec.

Some additional discussion can be found in the comments to #45.

Support of LHS/RHS expressions

RHS expressions are optional due to the specification by they could be practical. The grammar is already implemented in the version v10.0.x but the implementation of the Transformer may need further testing. Especially if we want to support the following optional rule's as well which is not obvious for me who should be implemented:

identifier <operator> identifier
constant <operator> constant

I think if we want a more elegant implementation we might have to change the grammar as well.

use examples from specs as resources

The current server uses mongo db and mapping of the structures stored in it because it stems from an implementation using it.
I would find it simpler to simply store the structures that need to be returned in a file, and even have one file that gets them simply from the specification.

The current implementation makes it a bit cumbersome, as result it was not updated together with the update of the spec (try to get /structures for example).

I looked into doing it, and I wrote an implementation using tinydb doing that.
It is a larger change so I wanted some feedback before investing time polishing it.

FastAPI v0.44.0 supports pydantic > 1.0.0

We should move to pydantic v1.

Irregular v0.2.0 publication to PyPI

During a test addition of a GitHub Actions workflow to publish tags on PyPI, I accidentally triggered a publication of v0.2.0 to PyPI via Travis.

We can delete the publication, but can then never publish the under the exact same version.
PyPI recommends releasing a .post1 version instead.

In the end, what do you think should be done?

Maybe it would even make sense to release a new major release (removing v0.2.0 from PyPI), since the work put into this repository since summer is backwards compatible. And it represents a full implementation of the OPTiMaDe spec v0.10.
Even though the spec is not at v1 yet, the implementation here is certainly at that level.

switch to pipenv?

currently, we have a list of required python packages in the setup.py (versions not fixed), and a set of fixed versions in requirements.txt and requirements-optional.txt.

One common issue is that even for fixed packages versions, the versions of dependencies of these packages can still change - this issue is addressed by pipenv.

It turns out we already have a Pipfile in the repository but it isn't mentioned anywhere.

Checklist for OPTiMaDe v0.10.1

@ml-evs: Hijacking this to list features/functionality that are still missing. For detailed discussion of each point, comment on the relevant issue or open one:

Server is missing /links endpoint

See 4.4 of spec.

add 2 examples of how to include documentation in python classes

test how to generate a client from the openapi.json

pick a language and generate a client
follow instructions in README to set up a server
check that the client can communicate with the server
write down the steps taken (e.g. in README) so that others can benefit

write a lark JSONTransformer / JSONdecoder

write a lark JSONTransformer/JSONdecoder that can take a query string and transform it into a JSON representation of the abstract syntax tree

type is missing in response

httptools dependency has build issues on GCC/Linux

Just in case anyone else runs into this, pip installing the repository currently leads to build issues with GCC 8.3/4.9. I've opened an issue to see if this can be rectified MagicStack/httptools/issues/36. My current workaround is to use clang when installing: CC=clang CXX=clang pip install ..

Minor changes to specification v0.10.1-develop

There are currently minor changes to the specification of v0.10.1-develop.
More may come and should be added below.

Schema / Models

Change from homepage_url to homepage (Materials-Consortia/OPTIMADE#238)
- Fixed by #127
A BaseResource for relationships may contain a meta field alongside id and type, with the field description (Materials-Consortia/OPTIMADE#232)
- Fixed by #131
Allow only float or None values for the lattice_vector content (within the list of lists), (Materials-Consortia/OPTIMADE#231)
- Fixed by #126
Add homepage_url field to LinksResourceAttributes (Materials-Consortia/OPTIMADE#228)
- Fixed by #127
New REQUIRED levels of response properties (#114)
- Note: This affects querying as well as the schema requirements.
- Fixed by #153

Grammar

Update definition of LengthComparison (Materials-Consortia/OPTIMADE#221)
- Fixed by #112
No constant <operator> constant comparisons allowed for strings (Materials-Consortia/OPTIMADE#229)
- Fixed by #112

Miscellaneous

OPTIONAL: Implement a landing page for requests to the base URL (Materials-Consortia/OPTIMADE#227)
Update the base URL from where the server is served (#122)
- Fixed by #155

generate static documentation on github from openapi.json

Move run.sh to a python file to be environment-agnostic

It may be prudent to move from the current run.sh file to a run.py similar to this one written by @fekad.

This will make it easier to develop in a Windows environment as well, but mostly simply remove any obvious OS environment dependency, although it may still be present indirectly.

Handle `include` standard JSON API query parameter

See OPTiMaDe spec repo issues #212 and #183, and look out for an upcoming PR.

The PR has been merged and was Materials-Consortia/OPTIMADE#219.

We should handle the JSON API query parameter include.

Implement `warnings`

It would be cool to add something similar to the Exception handlers for Warnings.

Perhaps it could be done with middleware? Or something similar.

What I have in mind is to catch from warnings import warn warnings and add them, as per the OPTiMaDe spec, under meta.warnings.

Central place with a list of queries for transformer tests

This is most definitely for another PR, but as a general question (to whomever happens to read this), would it not be beneficial for future parser developers/contributors that we have a central place with a list of queries, wherein one could also add their parser's expected output for testing? E.g. a python dictionary or importable JSON file?

_Originally posted by @CasperWA in #66 (comment)

Response by @fekad:

That's a good idea. But rather having a dictionary I would prefer a python template file which contains all the cases. The dictionary/JSON can easily become quite complex when you start to categorise the test cases or you want to test failures with different exceptions etc...

Originally posted by @fekad in #66 (comment)

Missing support for timestamps/datetime in grammar

The grammar is currently missing support for timestamps (or datetime.datetime in terms of Python types).

The relevant section in the spec may be found here.

Specifically we need to recognize timestamps as values being of the form specified by RFC 3339 Internet Date/Time Format and then turn them into datetime.datetime objects if they are OK, otherwise, the implementation should return a 400 Bad Request.

Implement relationships in server

Relationships should be implemented in the server.

The generic example involves a structure with a related reference.
These references may also then be provided in the top-level included field.

Developer-friendly pre-commit openapi.json visual diff

Related to #4. @waychal is looking into https://github.com/quen2404/openapi-diff (linked to in https://openapi.tools/#miscellaneous). This will help folks evaluate what they are changing in this file.

Implement optional `implementation` in top-level meta response

While the implementation field is OPTIONAL in the response for the top-level meta field, it would still be nice to have the implementation version as well as the URL to this GitHub repository in the test server.

setuptools package with server as "extra"

Great work on the optimade tools.

Unfortunately, the server implementation brings a lot of dependencies (including Python 3.7?). This way, I cannot use the package out of the box (i.e. git repo) and have to either clone/copy code, or modify the setup, etc.

Would be great, if this had more flexibility and we could use an additional "extra" (similar to dev, e.g. pip install optimade[server]) to make it more modular.

If you agree, I could try to find the time and prepare pull request: #63

Commented-out validator

The issue is class-inheritance.
This validator is run for sub-classes as well, as was intended, but it doesn't register and recognize the instance of the sub-class as that: An instance of the sub-class. Instead it thinks it's an instance of the parent class and fails.
In the pydantic docs it's writen that this can be avoided by passing the check_fields=False to the validator. But this is for pydantic v1, and didn't seem to work for me when I tested it here.
So I have commented it out, since it should be able to work for pydantic v1.

I don't know if this makes this PR blocked until we have upgraded or not, but it definitely not desireable to have a commented out model validator.

Originally posted by @CasperWA in #103

This is most optimally fixed in a PR that moves this repo to pydantic v1.

List properties and HAS _ operators missing

This is known, but just so we have an issue for it, we are still missing this functionality. A discussion was started in @fekad's PR.

Required in the spec:

HAS ANY
HAS ALL

Optional:

HAS ONLY
Search over correlated data e.g. list1:list2:... HAS val1:val2:...

Use services for testing/updating dependencies?

I've just restarted the latest travis build and the build is broken:
https://travis-ci.org/Materials-Consortia/optimade-python-tools/jobs/617866129?utm_medium=notification&utm_source=github_status

The latest version of fastapi installs pydantic 1.1.1 (instead of pydantic 0.x like before), which breaks the interface.

Since pydantic is used directly in the python tools, I think it is anyhow good practice to depend on it directly.

Create "special" index meta-database server

Since this is something all providers, who want to join the providers.json in the OPTiMaDe specification repository need to have, I suggest we create a special instance of the server that only serves the appropriate endpoints (/links and /info), with a JSON file that can be altered for each implementation provider.

Essentially, it will utilize the enhancements introduced by PRs #95 and #99 to serve specific endpoints and load a JSON file similarly to how we currently load test entries.

This server may be started with a separate bash script or by editing the script so that one can pass a desire to switch to a different FastAPI application.

Enforce use of autoformatter

I'd suggest enforcing the use of black when updating the Python Open API definitions, to insure consistency and readability.

Implement LENGTH in query

It is easy to match ("=") the size of an array in MongoDB but it is not obvious to use the size of an array in other comparisons.
As far as I can see there are two options:

we can use $where which is slow,
we can store the size of the arrays explicitly in the database. To avoid any collision we could use a postfix like: _SIZE. This should unique because all the properties are lowercase.

server.jsonapi has no patternProperties

In the current version of pydanitc patternProperties can only be added inside sub-elements of properties. If that is fixed then remove items from optimade.server.models.jsonapi.Attributes and optimade.server.models.jsonapi.Realationships and make it match that new formalism.

Somewhere in the creation of the openapi.json file the patternProperties is lost (likely something is using an old version of pydantic that converts a Dict object to object if the key is not a string).

The `sortable` field must be added to models

The sortable field should be added to the EntryInfoProperty model.

Spec. reference

PyPI release checklist

Hi all, we should prepare a PyPI release once we're up to date with v0.10.0 of the spec, as the paper approaches a final draft.

This issue is intended to track packaging changes before the release, and NOT missing features/functionality of the release, which we can track at #29.

Currently it looks like @dwinston is the maintainer on PyPI, so we might need to get that changed (not sure if Donny is still following here, so we should reach out to him)?
Should we align PyPI major.minor version numbers with the specification? A third number can be free to track hotfixes to both the server and any changes that reflect minor fixes to the spec.
We should decide which versions of dependencies to lock in, the important ones being Lark, FastAPI and pydantic.
Should we switch dependency handling to pipenv? #37

add one-line command to update openapi.json

add a one-line command to update optimade.json from python classes
make sure, json is indented to 2 spaces
possibly add pre-commit hook that runs the command

disable serving API under /v0.10 and /v0.10.0 by default?

Currently, optimade-python-tools serves the same API under three separate base urls:

/optimade/
/optimade/vMAJ.MIN
/optimade/vMAJ.MIN.PAT

Besides bloating the openapi.json file and the corresponding swagger documentation, I would argue this also creates an expectation on the client side that the API will remain available under these base urls.
However - if I understand correctly - in the current implementation of the optimade-python-tools this is not foreseen. E.g. if we update the models for spec v0.10.1, then the /optimade/v0.10.0 base url will be gone.

To me this suggests that the default behavior of the server should be to serve only under /optimade, while making it easy for developers to replicate the API under additional base urls if they wish to do so.

We should probably make all AttributeErrors ValueErrors?

Add more tests

We should add tests for:

materials-consortia / optimade-python-tools Goto Github PK

optimade-python-tools's People

Contributors

Stargazers

Watchers

Forkers

optimade-python-tools's Issues

Schema / Models

Grammar

Miscellaneous

Recommend Projects

Recommend Topics

Recommend Org