Interoperability with MMSchema #1103
Conversation
|
PR subject to change based on how multi-conformers are handled. See relevant issue. |
| if isinstance(atom1, (int, np.integer)) and isinstance( | ||
| atom2, (int, np.integer) | ||
| ): |
There was a problem hiding this comment.
This is throwing a deprecation warning for np.integer when I try it locally. Should an upstream component be updated so that np.integer is no longer needed?
There was a problem hiding this comment.
Are you sure the deprecation warning is coming from np.integer and not np.int (somewhere else)? The latter was deprecated in v1.20.0. Anyway, this can be easily moved to the mmic_openff pkg.
There was a problem hiding this comment.
isinstance(atom1, numbers.Integral) should be enough, see: https://docs.python.org/3/library/numbers.html numpy/numpy#4547
There was a problem hiding this comment.
Thanks for opening this, @andrew-abimansour. I poked around the MolSSI ecosystem docs a bit, and noted some feedback here. We'll probably eventually want to get the deps installed so tests can run on the docstrings (and so we can add more pointed tests if appropriate), though I'm open to alternatives if you'd like this to be merged sooner.
| version : int, default=None | ||
| MMSchema version. Overrides schema_object.schema_version. |
There was a problem hiding this comment.
(non blocking) Will this always be an int, or might it need to support more general values, like a packaging.version object?
openff/toolkit/topology/molecule.py
Outdated
| Create Molecule from MMElemental Molecule: | ||
|
|
||
| >>> import mmelemental | ||
| >>> mm_mol = mmelemental.models.Molecule(symbols=["C", "C"]) |
There was a problem hiding this comment.
(blocking) This is an invalid OpenFF molecule because it's not a valid kekule structure (doesn't have any bonds). It may not crash now, but it will eventually, so it may be best to do a molecule like water where we can easily fill in bonds.
|
Closing as stale, but we would happily accept an updated PR with similar aims. Until version 0.11.0 is cut, please point new PRs to the |
This PR enables interoperability with MMElemental for the FrozenMolecule class.