Do not allow unique_molecules=None when required

docs/releasehistory.md

@@ -10,6 +10,8 @@ Releases follow the `major.minor.micro` scheme recommended by [PEP440](https://w
 
 ### API-breaking changes
 
+- [PR #1742](https://github.com/openforcefield/openff-toolkit/pull/1742): `Topology.from_openmm` and `Topology.from_mdtraj` now require `unique_molecules` at the level of the function signature. Previously `None` was allowed but would always throw `MissingUniqueMoleculesError`, which is now removed.
+
 ### Behavior changes
 
 ### Bugfixes

openff/toolkit/_tests/test_topology.py

@@ -64,7 +64,6 @@
     InvalidBoxVectorsError,
     InvalidPeriodicityError,
     MissingConformersError,
-    MissingUniqueMoleculesError,
     MoleculeNotInTopologyError,
     NonUniqueSubstructureName,
     SubstructureAtomSmartsInvalid,
@@ -507,11 +506,6 @@ def test_from_openmm(self):
             get_data_file_path("systems/packmol_boxes/cyclohexane_ethanol_0.4_0.6.pdb")
         )
 
-        with pytest.raises(
-            MissingUniqueMoleculesError, match="requires a list of Molecule objects"
-        ):
-            Topology.from_openmm(pdbfile.topology)
-
         molecules = [create_ethanol(), create_cyclohexane()]
 
         topology = Topology.from_openmm(
@@ -529,6 +523,13 @@ def test_from_openmm(self):
         for omm_atom, off_atom in zip(pdbfile.topology.atoms(), topology.atoms):
             assert omm_atom.name == off_atom.name
 
+    def test_from_openmm_missing_unique_molecules(self):
+        with pytest.raises(
+            TypeError,
+            match="missing 1 required positional argument: 'unique_molecules'",
+        ):
+            Topology.from_openmm(Molecule.from_smiles("O").to_topology().to_openmm())
+
     def test_from_openmm_virtual_sites(self):
         from openff.toolkit import ForceField
 
@@ -883,7 +884,8 @@ def test_from_mdtraj(self):
         trj = md.load(pdb_path)
 
         with pytest.raises(
-            MissingUniqueMoleculesError, match="requires a list of Molecule objects"
+            TypeError,
+            match="missing 1 required positional argument: 'unique_molecules'",
         ):
             Topology.from_mdtraj(trj.top)
 

openff/toolkit/topology/__init__.py

@@ -12,7 +12,6 @@
     ImproperDict,
     InvalidBoxVectorsError,
     InvalidPeriodicityError,
-    MissingUniqueMoleculesError,
     NotBondedError,
     SortedDict,
     TagSortedDict,
@@ -31,7 +30,6 @@
     "ImproperDict",
     "InvalidBoxVectorsError",
     "InvalidPeriodicityError",
-    "MissingUniqueMoleculesError",
     "NotBondedError",
     "SortedDict",
     "TagSortedDict",

openff/toolkit/topology/topology.py

@@ -58,7 +58,6 @@
     InvalidBoxVectorsError,
     InvalidPeriodicityError,
     MissingConformersError,
-    MissingUniqueMoleculesError,
     MoleculeNotInTopologyError,
     NotBondedError,
     VirtualSitesUnsupportedError,
@@ -1320,7 +1319,7 @@ def _openmm_topology_to_networkx(openmm_topology):
     def from_openmm(
         cls,
         openmm_topology: "openmm.app.Topology",
-        unique_molecules: Optional[Iterable[FrozenMolecule]] = None,
+        unique_molecules: Iterable[FrozenMolecule],
         positions: Union[None, Quantity, "OMMQuantity"] = None,
     ) -> "Topology":
         """
@@ -1340,9 +1339,9 @@ def from_openmm(
 
         Parameters
         ----------
-        openmm_topology
+        openmm_topology: openmm.app.Topology
             The OpenMM Topology object to convert
-        unique_molecules
+        unique_molecules: Iterable[FrozenMolecule]
             An iterable containing all the unique molecules in the topology.
             This is used to identify the molecules in the OpenMM topology and
             provide any missing chemical information. Each chemical species in
@@ -1352,19 +1351,17 @@ def from_openmm(
             these reference molecules to the molecules appearing in the
             topology. If bond orders are specified in the topology, these will
             be used in matching as well.
-        positions
+        positions: optional, openff.unit.Quantity or openmm.unit.Quantity
             Positions for the atoms in the new topology.
 
         Returns
         -------
-        topology
+        topology: openff.toolkit.Topology
             An OpenFF Topology object, constructed from the molecules in
             ``unique_molecules``, with the same atom order as the input topology.
 
         Raises
         ------
-        MissingUniqueMoleculesError
-            If ``unique_molecules`` is ``None``
         DuplicateUniqueMoleculeError
             If the same connectivity graph is represented by two different
             molecules in ``unique_molecules``
@@ -1382,12 +1379,6 @@ def from_openmm(
             if omm_bond.order is None:
                 omm_has_bond_orders = False
 
-        if unique_molecules is None:
-            raise MissingUniqueMoleculesError(
-                "Topology.from_openmm requires a list of Molecule objects "
-                "passed as unique_molecules, but None was passed."
-            )
-
         # Convert all unique mols to graphs
         topology = cls()
         graph_to_unq_mol: dict[Graph, FrozenMolecule] = {}
@@ -1546,7 +1537,7 @@ def _ensure_unique_atom_names(self, ensure_unique_atom_names: Union[str, bool]):
     def from_pdb(
         cls,
         file_path: Union[str, Path, TextIO],
-        unique_molecules: Optional[Iterable[Molecule]] = None,
+        unique_molecules: Iterable[Molecule] = tuple(),
         toolkit_registry=GLOBAL_TOOLKIT_REGISTRY,
         _custom_substructures: Optional[dict[str, list[str]]] = None,
         _additional_substructures: Optional[Iterable[Molecule]] = None,
@@ -1624,9 +1615,9 @@ def from_pdb(
         ----------
         file_path : str, Path, or file object
             PDB information to be passed to OpenMM PDBFile object for loading
-        unique_molecules : Iterable of Molecule. Default = None
+        unique_molecules : Iterable of Molecule. Default = tuple()
             OpenFF Molecule objects corresponding to the molecules in the input
-            PDB. See above for details.
+            PDB. Can be empty. See above for details.
         toolkit_registry : ToolkitRegistry. Default = None
             The ToolkitRegistry to use as the cheminformatics backend.
         _custom_substructures: dict[str, list[str]], Default = {}
@@ -2128,7 +2119,7 @@ def set_positions(self, array: Quantity):
     def from_mdtraj(
         cls,
         mdtraj_topology: "mdtraj.Topology",
-        unique_molecules: Optional[Iterable[MoleculeLike]] = None,
+        unique_molecules: Iterable[FrozenMolecule],
         positions: Union[None, "OMMQuantity", Quantity] = None,
     ):
         """
@@ -2144,9 +2135,9 @@ def from_mdtraj(
 
         Parameters
         ----------
-        mdtraj_topology
+        mdtraj_topology: mdtraj.Topology
             The MDTraj Topology object to convert
-        unique_molecules
+        unique_molecules: Iterable[FrozenMolecule]
             An iterable containing all the unique molecules in the topology.
             This is used to identify the molecules in the MDTraj topology and
             provide any missing chemical information. Each chemical species in
@@ -2156,19 +2147,17 @@ def from_mdtraj(
             these reference molecules to the molecules appearing in the
             topology. If bond orders are specified in the topology, these will
             be used in matching as well.
-        positions
+        positions: optional, openff.unit.Quantity or openmm.unit.Quantity
             Positions for the atoms in the new topology.
 
         Returns
         -------
-        topology
+        topology: openff.toolkit.Topology
             An OpenFF Topology object, constructed from the molecules in
             ``unique_molecules``, with the same atom order as the input topology.
 
         Raises
         ------
-        MissingUniqueMoleculesError
-            If ``unique_molecules`` is ``None``
         DuplicateUniqueMoleculeError
             If the same connectivity graph is represented by two different
             molecules in ``unique_molecules``

openff/toolkit/utils/exceptions.py

@@ -201,12 +201,6 @@ class InvalidPeriodicityError(OpenFFToolkitException):
     """
 
 
-class MissingUniqueMoleculesError(OpenFFToolkitException):
-    """
-    Exception for when unique_molecules is required but not found
-    """
-
-
 class SMIRKSMismatchError(OpenFFToolkitException):
     """
     Exception for cases where smirks are inappropriate