Remove dormant API points (#1058)

* Remove dead API points

* Remove _networkx_to_hill_formula, replace _to_mdtraj, update release notes

* Fix typo

Author: mattwthompson

docs/releasehistory.md

@@ -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
 

openff/toolkit/tests/test_forcefield.py

@@ -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
     )

openff/toolkit/topology/topology.py

@@ -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):
         """
@@ -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.

openff/toolkit/typing/engines/smirnoff/forcefield.py

@@ -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):