diff --git a/docs/releasehistory.md b/docs/releasehistory.md index 9261976d0..3ddbca5e4 100644 --- a/docs/releasehistory.md +++ b/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 diff --git a/openff/toolkit/_tests/test_topology.py b/openff/toolkit/_tests/test_topology.py index 856a24255..d2385a40c 100644 --- a/openff/toolkit/_tests/test_topology.py +++ b/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) diff --git a/openff/toolkit/topology/__init__.py b/openff/toolkit/topology/__init__.py index 2c7a1aa24..9796f1aa1 100644 --- a/openff/toolkit/topology/__init__.py +++ b/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", diff --git a/openff/toolkit/topology/topology.py b/openff/toolkit/topology/topology.py index 4a9b3e37c..b137583c0 100644 --- a/openff/toolkit/topology/topology.py +++ b/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`` diff --git a/openff/toolkit/utils/exceptions.py b/openff/toolkit/utils/exceptions.py index e7709fbe3..16efbd13f 100644 --- a/openff/toolkit/utils/exceptions.py +++ b/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