search

openforcefield / openff-models

Helper classes for Pydantic compatibility in the OpenFF stack
MIT License
3 stars 2 forks source link

support nested models #9

Open mikemhenry opened 2 years ago

mikemhenry commented 2 years ago

Fixes the issue with nested models. Will add some tests and more documentation as well. The tl;dr is first we do a pass to convert the json to a dictionary, then we unitify things.

mikemhenry commented 2 years ago

but let me know @mattwthompson what you think of this approach.

mikemhenry commented 1 year ago

So this solves the first half of https://github.com/mattwthompson/openff-models/issues/8 but I will make another PR that optionally enforces a more strict unit check.

mikemhenry commented 1 year ago

@mattwthompson I will be adding tests to this PR ASAP! Is there anything else you would like to see? It is pretty old, we have been using it for awhile now in gufe and I would like to get it merged in sometime within the next two weeks.

mattwthompson commented 1 year ago

Tests should be fine here, there might be some more steps to getting it in a release (though once it looks good the release process for this package is quick). I might want to tinker with large models to uncover any obvious performance bottlenecks, but that doesn't need to be included here. Eventually I'd like to be able to store i.e. JSONs of parametrized systems of millions of atoms. Is gufe using this for anything large?

I use this in Interchange now so it brushes up against parts of our stack that needs to be stable so I might want to run some integration tests before putting it into a release. The toolkit doesn't use this functionality, though, so I don't imagine any trouble ahead there.