Deprecate older PDB loading methods (#1716)

* Deprecate older PDB loading methods * Apply suggestions from code review --------- Co-authored-by: Jeff Wagner <[email protected]>
openforcefield · Oct 5, 2023 · d296ade · d296ade
1 parent c017ee3
commit d296ade
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 2 deletions.
diff --git a/docs/releasehistory.md b/docs/releasehistory.md
@@ -6,12 +6,16 @@ Releases follow the `major.minor.micro` scheme recommended by [PEP440](https://w
 * `minor` increments add features but do not break API compatibility
 * `micro` increments represent bugfix releases or improvements in documentation
 
+
+
 ## 0.14.4 
 
 ### Behavior changes
 
 - [PR #1705](https://github.com/openforcefield/openff-toolkit/pull/1705): Do not raise warning when `allow_undefined_stereo=True`.
 - [PR #1695](https://github.com/openforcefield/openff-toolkit/pull/1695): `ChemicalEnvironmentParsingError` is now raised when an underlying toolkit fails to parse a SMARTS/SMIRKS pattern it is given during substructure matching.
+- [PR #1716](https://github.com/openforcefield/openff-toolkit/pull/1716): Adds deprecation warnings to `Molecule.from_polymer_pdb`, `Molecule.from_pdb_and_smiles`, and `RDKitToolkitWrapper.from_pdb_and_smiles` instead pointing users toward `Topology.from_pdb`.
+
 
 ### Bugfixes
 

diff --git a/openff/toolkit/topology/molecule.py b/openff/toolkit/topology/molecule.py
@@ -96,14 +96,17 @@
 
 # TODO: Can we have the `ALLOWED_*_MODELS` list automatically appear in the docstrings below?
 # TODO: Should `ALLOWED_*_MODELS` be objects instead of strings?
-# TODO: Should these be imported from `openff.toolkit.cheminformatics.aromaticity_models` and `.bondorder_models`?
 
 # TODO: Allow all OpenEye aromaticity models to be used with OpenEye names?
 #       Only support OEAroModel_MDL in RDKit version?
 
 TKR: TypeAlias = Union[ToolkitRegistry, ToolkitWrapper]
 
 
+class MoleculeDeprecationWarning(UserWarning):
+    """Warning for deprecated portions of the Molecule API."""
+
+
 class Particle(Serializable):
     """
     Base class for all particles in a molecule.
@@ -3930,6 +3933,13 @@ def from_polymer_pdb(
         import openmm.unit as openmm_unit
         from openmm.app import PDBFile
 
+        warnings.warn(
+            "`Molecule.from_polymer_pdb` is deprecated in favor of `Topology.from_pdb`, the recommended "
+            "method for loading PDB files. This method will be removed in a future release of the OpenFF Toolkit.",
+            MoleculeDeprecationWarning,
+            stacklevel=2,
+        )
+
         if isinstance(toolkit_registry, ToolkitWrapper):
             toolkit_registry = ToolkitRegistry([toolkit_registry])
 
@@ -4757,6 +4767,12 @@ def from_pdb_and_smiles(
         InvalidConformerError
             If the SMILES and PDB molecules are not isomorphic.
         """
+        warnings.warn(
+            "`Molecule.from_pdb_and_smiles` is deprecated in favor of `Topology.from_pdb`, the recommended "
+            "method for loading PDB files. This method will be removed in a future release of the OpenFF Toolkit.",
+            MoleculeDeprecationWarning,
+            stacklevel=2,
+        )
 
         toolkit = RDKitToolkitWrapper()
         return toolkit.from_pdb_and_smiles(

diff --git a/openff/toolkit/utils/rdkit_wrapper.py b/openff/toolkit/utils/rdkit_wrapper.py
@@ -11,6 +11,7 @@
 import logging
 import pathlib
 import tempfile
+import warnings
 from collections import defaultdict
 from typing import TYPE_CHECKING, Dict, List, Optional, Tuple
 
@@ -222,9 +223,15 @@ def from_pdb_and_smiles(
         ------
         InvalidConformerError : if the SMILES and PDB molecules are not isomorphic.
         """
-
         from rdkit import Chem
 
+        warnings.warn(
+            "`RDKitToolkitWrapper.from_polymer_pdb` is deprecated in favor of `Topology.from_pdb`, the recommended "
+            "method for loading PDB files. (Note the `unique_molecules` argument.) This method will "
+            "be removed in a future release of the OpenFF Toolkit.",
+            stacklevel=2,
+        )
+
         # Make the molecule from smiles
         offmol = self.from_smiles(
             smiles,