Skip to content

Commit

Permalink
Remove dormant API points (#1058)
Browse files Browse the repository at this point in the history
* Remove dead API points

* Remove _networkx_to_hill_formula, replace _to_mdtraj, update release notes

* Fix typo
  • Loading branch information
mattwthompson committed Sep 2, 2021
1 parent 881c29a commit 6463e8a
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 140 deletions.
4 changes: 4 additions & 0 deletions docs/releasehistory.md
Expand Up @@ -36,6 +36,10 @@ Releases follow the `major.minor.micro` scheme recommended by [PEP440](https://w
created through the [`Molecule.to_rdkit()`](openff.toolkit.topology.Molecule.to_rdkit)
method have the `NoImplicit` property set to `True` on all atoms. This prevents RDKit from
incorrectly adding hydrogen atoms to to molecule.
- [PR #1058](https://github.com/openforcefield/openforcefield/pull/1058): Removes the unimplemented methods
[`ForceField.create_parmed_structure`](openff.toolkit.typing.engines.smirnoff.ForceField.create_parmed_structure),
[`Topology.to_parmed`](openff.toolkit.topology.Topology.to_parmed), and
[`Topology.from_parmed`](openff.toolkit.topology.Topology.from_parmed).

### Tests updated

Expand Down
21 changes: 0 additions & 21 deletions openff/toolkit/tests/test_forcefield.py
Expand Up @@ -1641,27 +1641,6 @@ def test_parameterize_mol_missing_stereo_openeye(self):
force_field = ForceField("test_forcefields/test_forcefield.offxml")
force_field.create_openmm_system(topology, toolkit_registry=toolkit_registry)

@pytest.mark.skip(
reason="We will not support going directly to ParmEd for now."
"We will instead feed OpenMM System objects to ParmEd "
"for further processing."
)
def test_parameterize_ethanol_to_parmed(self):
from simtk.openmm import app

forcefield = ForceField("test_forcefields/test_forcefield.offxml")
pdbfile = app.PDBFile(get_data_file_path("systems/test_systems/1_ethanol.pdb"))
# toolkit_wrapper = RDKitToolkitWrapper()
molecules = [
Molecule.from_file(get_data_file_path(name))
for name in ("molecules/ethanol.mol2",)
]
topology = Topology.from_openmm(pdbfile.topology, unique_molecules=molecules)

parmed_system = forcefield.create_parmed_structure(
topology, positions=pdbfile.getPositions()
)

@pytest.mark.parametrize(
"toolkit_registry,registry_description", toolkit_registries
)
Expand Down
103 changes: 4 additions & 99 deletions openff/toolkit/topology/topology.py
Expand Up @@ -2110,57 +2110,6 @@ def from_dict(cls, d):
# Implement abstract method Serializable.to_dict()
raise NotImplementedError() # TODO

# TODO: Merge this into Molecule.from_networkx if/when we implement that.
# TODO: can we now remove this as we have the ability to do this in the Molecule class?
@staticmethod
def _networkx_to_hill_formula(mol_graph):
"""
Convert a networkX representation of a molecule to a molecule formula. Used in printing out
informative error messages when a molecule from an openmm topology can't be matched.
Parameters
----------
mol_graph : a networkX graph
The graph representation of a molecule
Returns
-------
formula : str
The molecular formula of the graph molecule
"""
from simtk.openmm.app import Element

# Make a flat list of all atomic numbers in the molecule
atom_nums = []
for idx in mol_graph.nodes:
atom_nums.append(mol_graph.nodes[idx]["atomic_number"])

# Count the number of instances of each atomic number
at_num_to_counts = dict([(unq, atom_nums.count(unq)) for unq in atom_nums])

symbol_to_counts = {}
# Check for C and H first, to make a correct hill formula (remember dicts in python 3.6+ are ordered)
if 6 in at_num_to_counts:
symbol_to_counts["C"] = at_num_to_counts[6]
del at_num_to_counts[6]

if 1 in at_num_to_counts:
symbol_to_counts["H"] = at_num_to_counts[1]
del at_num_to_counts[1]

# Now count instances of all elements other than C and H, in order of ascending atomic number
sorted_atom_nums = sorted(at_num_to_counts.keys())
for atom_num in sorted_atom_nums:
symbol_to_counts[
Element.getByAtomicNumber(atom_num).symbol
] = at_num_to_counts[atom_num]

# Finally format the formula as string
formula = ""
for ele, count in symbol_to_counts.items():
formula += f"{ele}{count}"
return formula

@classmethod
def from_openmm(cls, openmm_topology, unique_molecules=None):
"""
Expand Down Expand Up @@ -2477,65 +2426,21 @@ def from_mdtraj(mdtraj_topology, unique_molecules=None):
mdtraj_topology.to_openmm(), unique_molecules=unique_molecules
)

# TODO: Jeff prepended an underscore on this before 0.2.0 release to remove it from the API.
# Before exposing this, we should look carefully at the information that is preserved/lost during this
# conversion, and make it clear what would happen to this information in a round trip. For example,
# we should know what would happen to formal and partial bond orders and charges, stereochemistry, and
# multi-conformer information. It will be important to document these risks to users, as all of these
# factors could lead to unintended behavior during system parameterization.
# Avoid removing this method, even though it is private and would not be difficult for most
# users to replace. Also avoid making it public as round-trips with MDTraj are likely
# to not preserve necessary information.
def _to_mdtraj(self):
"""
Create an MDTraj Topology object.
Returns
----------
mdtraj_topology : mdtraj.Topology
An MDTraj Topology object
#"""
"""
import mdtraj as md

return md.Topology.from_openmm(self.to_openmm())

@staticmethod
def from_parmed(parmed_structure, unique_molecules=None):
"""
.. warning:: This functionality will be implemented in a future toolkit release.
Construct an OpenFF Topology object from a ParmEd Structure object.
Parameters
----------
parmed_structure : parmed.Structure
A ParmEd structure object
unique_molecules : iterable of objects that can be used to construct unique Molecule objects
All unique molecules must be provided, in any order, though multiple copies of each molecule are allowed.
The atomic elements and bond connectivity will be used to match the reference molecules
to molecule graphs appearing in the structure's ``topology`` object. If bond orders are present in the
structure's ``topology`` object, these will be used in matching as well.
Returns
-------
topology : openff.toolkit.topology.Topology
An OpenFF Topology object
"""
# TODO: Implement functionality
raise NotImplementedError

def to_parmed(self):
"""
.. warning:: This functionality will be implemented in a future toolkit release.
Create a ParmEd Structure object.
Returns
----------
parmed_structure : parmed.Structure
A ParmEd Structure objecft
"""
# TODO: Implement functionality
raise NotImplementedError

# TODO: Jeff prepended an underscore on this before 0.2.0 release to remove it from the API.
# This function is deprecated and expects the OpenEye toolkit. We need to discuss what
# to do with this functionality in light of our move to the ToolkitWrapper architecture.
Expand Down
20 changes: 0 additions & 20 deletions openff/toolkit/typing/engines/smirnoff/forcefield.py
Expand Up @@ -1370,26 +1370,6 @@ def create_openmm_system(self, topology, **kwargs):
else:
return system

def create_parmed_structure(self, topology, positions, **kwargs):
"""Create a ParmEd Structure object representing the interactions for the specified Topology with the current force field
This method creates a `ParmEd <http://github.com/parmed/parmed>`_ ``Structure`` object containing a topology, positions, and parameters.
Parameters
----------
topology : openff.toolkit.topology.Topology
The ``Topology`` corresponding to the OpenMM ``System`` object to be created.
positions : simtk.unit.Quantity of dimension (natoms,3) with units compatible with angstroms
The positions corresponding to the ``System`` object to be created
Returns
-------
structure : parmed.Structure
The newly created ``parmed.Structure`` object
"""
raise NotImplementedError

@requires_package("openff.interchange")
@requires_package("mdtraj")
def _to_interchange(self, topology, box=None):
Expand Down

0 comments on commit 6463e8a

Please sign in to comment.