Stop OpenEyeToolkitWrapper from erroring on planar trivalent nitrogens if they have trivalent carbon neighbors

docs/releasehistory.md

@@ -12,6 +12,7 @@ Releases follow the `major.minor.micro` scheme recommended by [PEP440](https://w
 
 ### Behavior changes
 
+- [PR #2231](https://github.com/openforcefield/openff-toolkit/pull/2231): Makes `OpenEyeToolkitWrapper.from_openeye` not raise an undefined-stereo error/warning for planar Ns if they have at least one trivalent carbon neighbor (This isn't a perfect solution but does handle a large fraction of the annoying over-policing of nitrogen stereo by our implementation in OpenEyeToolkitWrapper). Resolves [Issue #1547](https://github.com/openforcefield/openff-toolkit/issues/1547), [Issue #725](https://github.com/openforcefield/openff-toolkit/issues/725), and [Issue #626](https://github.com/openforcefield/openff-toolkit/issues/626).
 - [PR #2238](https://github.com/openforcefield/openff-toolkit/pull/2238): Adds support for QCSchema V2 in `Molecule.to_qcschema`, which now only returns QCSchema V2 models.
 
 ### Bugfixes

openff/toolkit/_tests/test_molecule.py

@@ -228,17 +228,11 @@ def mini_drug_bank(xfail_mols=None, wip_mols=None):
 # All the molecules that raise UndefinedStereochemistryError when read by OETK()
 openeye_drugbank_undefined_stereo_mols = {
     "DrugBank_1634",
-    "DrugBank_1700",
     "DrugBank_1962",
     "DrugBank_2519",
-    "DrugBank_2987",
-    "DrugBank_3502",
     "DrugBank_3930",
-    "DrugBank_4161",
-    "DrugBank_4162",
     "DrugBank_5043",
     "DrugBank_5418",
-    "DrugBank_6531",
 }
 
 # All the molecules that raise UndefinedStereochemistryError when read by RDKTKW().
@@ -254,8 +248,6 @@ def mini_drug_bank(xfail_mols=None, wip_mols=None):
 }
 
 
-# Missing stereo in OE but not RDK:  'DrugBank_2987', 'DrugBank_3502', 'DrugBank_4161',
-# 'DrugBank_4162', 'DrugBank_6531', 'DrugBank_1700',
 drugbank_stereogenic_in_rdkit_but_not_openeye = {
     "DrugBank_5329",
     "DrugBank_7124",
@@ -1124,23 +1116,11 @@ def test_to_from_iupac(self):
         Test basic behavior of the IUPAC conversion functions. More rigorous
         testing of the toolkity wrapper behavior is in test_toolkits.py
         """
-        from openff.toolkit.utils.toolkits import (
-            InvalidIUPACNameError,
-            UndefinedStereochemistryError,
-        )
+        from openff.toolkit.utils.toolkits import InvalidIUPACNameError
 
         with pytest.raises(InvalidIUPACNameError):
             Molecule.from_iupac(".BETA.-PINENE")
 
-        # DrugBank_977, tagged as a problem molecule in earlier tests
-        bad_stereo_iupac = (
-            "(~{E},3~{R},5~{S})-7-[4-(4-fluorophenyl)-6-isopropyl-2-"
-            "[methyl(methylsulfonyl)amino]pyrimidin-5-yl]-3,5-"
-            "dihydroxy-hept-6-enoic acid"
-        )
-        with pytest.raises(UndefinedStereochemistryError):
-            Molecule.from_iupac(bad_stereo_iupac)
-
         cholesterol = Molecule.from_smiles(
             "C[C@H](CCCC(C)C)[C@H]1CC[C@@H]2[C@@]1(CC[C@H]3[C@H]2CC=C4[C@@]3(CC[C@@H](C4)O)C)C"
         )
@@ -1379,30 +1359,15 @@ def test_to_xyz_no_conformers(self):
     )
     def test_to_from_file(self, molecule, format):
         """Test that conversion/creation of a molecule to and from a file is consistent."""
-        from openff.toolkit.utils.toolkits import UndefinedStereochemistryError
-
         # TODO: Test all file capabilities; the current test is minimal
         # TODO: This is only for OE. Expand to both OE and RDKit toolkits.
-        # Molecules that are known to raise UndefinedStereochemistryError.
-        undefined_stereo_mols = {
-            "DrugBank_1700",
-            "DrugBank_2987",
-            "DrugBank_3502",
-            "DrugBank_4161",
-            "DrugBank_4162",
-            "DrugBank_6531",
-        }
-        undefined_stereo = molecule.name in undefined_stereo_mols
 
         # The file is automatically deleted outside the with-clause.
         with NamedTemporaryFile(suffix="." + format) as iofile:
             # If this has undefined stereo, check that the exception is raised.
             extension = os.path.splitext(iofile.name)[1][1:]
             molecule.to_file(iofile.name, extension)
-            if undefined_stereo:
-                with pytest.raises(UndefinedStereochemistryError):
-                    Molecule.from_file(iofile.name)
-            molecule2 = Molecule.from_file(iofile.name, allow_undefined_stereo=undefined_stereo)
+            molecule2 = Molecule.from_file(iofile.name)
             assert molecule == molecule2
             # TODO: Test to make sure properties are preserved?
             # NOTE: We can't read pdb files and expect chemical information to be preserved

openff/toolkit/_tests/test_toolkits.py

@@ -92,73 +92,55 @@ def get_mini_drug_bank(toolkit_class, xfail_mols=None):
     "DrugBank_5415",
     "DrugBank_5418",
     "DrugBank_2955",
-    "DrugBank_2987",
     "DrugBank_5555",
     "DrugBank_472",
     "DrugBank_5737",
     "DrugBank_3332",
     "DrugBank_3461",
     "DrugBank_794",
-    "DrugBank_3502",
     "DrugBank_6026",
     "DrugBank_3622",
-    "DrugBank_977",
     "DrugBank_3693",
     "DrugBank_3726",
     "DrugBank_3739",
     "DrugBank_6222",
-    "DrugBank_6232",
     "DrugBank_3844",
-    "DrugBank_6295",
     "DrugBank_6304",
     "DrugBank_6305",
     "DrugBank_3930",
     "DrugBank_6329",
     "DrugBank_6353",
     "DrugBank_6355",
     "DrugBank_6401",
-    "DrugBank_4161",
-    "DrugBank_4162",
     "DrugBank_6509",
-    "DrugBank_6531",
     "DrugBank_1570",
     "DrugBank_4249",
     "DrugBank_1634",
     "DrugBank_1659",
     "DrugBank_6647",
-    "DrugBank_1700",
     "DrugBank_1721",
-    "DrugBank_1742",
     "DrugBank_1802",
-    "DrugBank_6775",
     "DrugBank_1849",
-    "DrugBank_1864",
     "DrugBank_6875",
     "DrugBank_1897",
     "DrugBank_4593",
     "DrugBank_1962",
-    "DrugBank_4662",
     "DrugBank_7049",
     "DrugBank_4702",
     "DrugBank_2095",
     "DrugBank_4778",
-    "DrugBank_2141",
     "DrugBank_2148",
     "DrugBank_2178",
     "DrugBank_4865",
     "DrugBank_2208",
     "DrugBank_2210",
     "DrugBank_2276",
-    "DrugBank_4959",
-    "DrugBank_4964",
     "DrugBank_5043",
     "DrugBank_2429",
     "DrugBank_5076",
     "DrugBank_2465",
     "DrugBank_2519",
     "DrugBank_2538",
-    "DrugBank_5158",
-    "DrugBank_5176",
     "DrugBank_2592",
 ]
 
@@ -197,9 +179,7 @@ def get_mini_drug_bank(toolkit_class, xfail_mols=None):
 ]
 
 openeye_iupac_bad_stereo = [
-    "DrugBank_977",
     "DrugBank_1634",
-    "DrugBank_1700",
     "DrugBank_1962",
     "DrugBank_2148",
     "DrugBank_2178",
@@ -209,40 +189,31 @@ def get_mini_drug_bank(toolkit_class, xfail_mols=None):
     "DrugBank_2538",
     "DrugBank_2592",
     "DrugBank_2651",
-    "DrugBank_2987",
     "DrugBank_3332",
-    "DrugBank_3502",
     "DrugBank_3622",
     "DrugBank_3726",
     "DrugBank_3844",
     "DrugBank_3930",
-    "DrugBank_4161",
-    "DrugBank_4162",
     "DrugBank_4778",
     "DrugBank_4593",
-    "DrugBank_4959",
     "DrugBank_5043",
     "DrugBank_5076",
-    "DrugBank_5176",
     "DrugBank_5418",
     "DrugBank_5737",
     "DrugBank_5902",
-    "DrugBank_6295",
     "DrugBank_6304",
     "DrugBank_6305",
     "DrugBank_6329",
     "DrugBank_6355",
     "DrugBank_6401",
     "DrugBank_6509",
-    "DrugBank_6531",
     "DrugBank_6647",
     "DrugBank_390",
     "DrugBank_810",
     "DrugBank_4316",
     "DrugBank_4346",
     "DrugBank_7124",
     "DrugBank_2799",
-    "DrugBank_4662",
     "DrugBank_4865",
     "DrugBank_2465",
 ]
@@ -353,6 +324,81 @@ def test_smiles_missing_stereochemistry(self):
             else:
                 Molecule.from_smiles(smiles, toolkit_registry=toolkit_wrapper)
 
+    @pytest.mark.parametrize(
+        "smiles,expected_stereo,exception_regex",
+        [
+            # Trivalent N with a trivalent (aromatic) carbon neighbor: Unlike RDKit, OpenEye
+            # perceives this N as stereogenic, however actual molecular forces may make it
+            # planar. These first two cases ensures that either planar or pyramidal
+            # configurations are accepted.
+            ("C[N@](CC)c1c(Br)cccc1", ["R"], None),
+            ("CN(CC)c1c(Br)cccc1", [None], None),
+            # Trivalent N with no trivalent carbon neighbor: an ordinary stereocenter, so
+            # unspecified stereo here must still raise as normal.
+            ("C[N@](CC)CCC", ["R"], None),
+            ("CN(CC)CCC", [None], "unspecified stereochemistry"),
+            # Troger's base (bridgehead conjugated tertiary amine)
+            ("c1(ccc3c(c1)C[N@]4c2ccc(cc2C[N@]3C4)C)C", ["S", "R"], None),
+            pytest.param(
+                "c1(ccc3c(c1)CN4c2ccc(cc2CN3C4)C)C",
+                [None, None],
+                "unspecified stereochemistry",
+                marks=pytest.mark.xfail(
+                    strict=True,
+                    reason="Conjugated bridgehead N stereo isn't enforced (matches RDKit, see rdkit#7434, "
+                    "openff-toolkit#2231); would be nice to catch this in the future",
+                ),
+            ),
+        ],
+    )
+    def test_smiles_nitrogen_chirality_skip(self, smiles, expected_stereo, exception_regex):
+        """
+        Test that a trivalent nitrogen with a trivalent carbon neighbor does not trigger
+        UndefinedStereochemistryError when its stereochemistry is unspecified, while an
+        otherwise-equivalent nitrogen without such a neighbor still does.
+        """
+        toolkit_wrapper = OpenEyeToolkitWrapper()
+
+        if exception_regex is not None:
+            with pytest.raises(UndefinedStereochemistryError, match=exception_regex):
+                toolkit_wrapper.from_smiles(smiles)
+            molecule = toolkit_wrapper.from_smiles(smiles, allow_undefined_stereo=True)
+        else:
+            molecule = toolkit_wrapper.from_smiles(smiles)
+
+        nitrogen_stereo = [atom.stereochemistry for atom in molecule.atoms if atom.symbol == "N"]
+        assert nitrogen_stereo == expected_stereo
+
+    @pytest.mark.parametrize(
+        "filename,expected_stereo,exception_regex",
+        [
+            ("bromoaniline_pyramidal_n.sdf", "R", None),
+            ("bromoaniline_planar_n.sdf", None, None),
+            ("trialkylamine_pyramidal_n.sdf", "R", None),
+            ("trialkylamine_planar_n.sdf", None, "unspecified stereochemistry"),
+        ],
+    )
+    def test_from_file_nitrogen_chirality_skip(self, filename, expected_stereo, exception_regex):
+        """
+        Same as test_smiles_nitrogen_chirality_skip, but exercising the from_file/SDF-reading
+        code path, which perceives stereochemistry from 3D coordinates via OE3DToInternalStereo
+        rather than from SMILES parity.
+        """
+        toolkit_wrapper = OpenEyeToolkitWrapper()
+        file_path = get_data_file_path(f"molecules/{filename}")
+
+        if exception_regex is not None:
+            with pytest.raises(UndefinedStereochemistryError, match=exception_regex):
+                toolkit_wrapper.from_file(file_path, file_format="sdf")
+            molecules = toolkit_wrapper.from_file(file_path, file_format="sdf", allow_undefined_stereo=True)
+        else:
+            molecules = toolkit_wrapper.from_file(file_path, file_format="sdf")
+
+        nitrogen_stereo = [
+            atom.stereochemistry for molecule in molecules for atom in molecule.atoms if atom.symbol == "N"
+        ]
+        assert nitrogen_stereo == [expected_stereo]
+
     def test_openeye_from_smiles_radical(self):
         """Test that parsing an SMILES with a radical raises RadicalsNotSupportedError."""
         with pytest.raises(RadicalsNotSupportedError):
@@ -2020,6 +2066,47 @@ def test_smiles_missing_stereochemistry(self, smiles, exception_regex):
         else:
             Molecule.from_smiles(smiles, toolkit_registry=toolkit_wrapper)
 
+    @pytest.mark.parametrize(
+        "smiles",
+        [
+            "C[N@](CC)c1c(Br)cccc1",
+            "CN(CC)c1c(Br)cccc1",
+            "C[N@](CC)CCC",
+            "CN(CC)CCC",
+        ],
+    )
+    def test_smiles_nitrogen_chirality_not_perceived(self, smiles):
+        """
+        RDKitToolkitWrapper does not perceive stereochemistry for these trivalent nitrogens,
+        whether or not they have a trivalent carbon neighbor and whether or not the SMILES
+        specifies @/@@ -- unlike OpenEyeToolkitWrapper, see
+        TestOpenEyeToolkitWrapper.test_smiles_nitrogen_chirality_skip.
+        """
+        toolkit_wrapper = RDKitToolkitWrapper()
+        molecule = toolkit_wrapper.from_smiles(smiles)
+        assert all(atom.stereochemistry is None for atom in molecule.atoms if atom.symbol == "N")
+
+    @pytest.mark.parametrize(
+        "filename",
+        [
+            "bromoaniline_pyramidal_n.sdf",
+            "bromoaniline_planar_n.sdf",
+            "trialkylamine_pyramidal_n.sdf",
+            "trialkylamine_planar_n.sdf",
+        ],
+    )
+    def test_from_file_nitrogen_chirality_not_perceived(self, filename):
+        """
+        RDKitToolkitWrapper does not perceive stereochemistry for these trivalent nitrogens from
+        3D coordinates either -- unlike OpenEyeToolkitWrapper, see
+        TestOpenEyeToolkitWrapper.test_from_file_nitrogen_chirality_skip.
+        """
+        toolkit_wrapper = RDKitToolkitWrapper()
+        file_path = get_data_file_path(f"molecules/{filename}")
+        molecules = toolkit_wrapper.from_file(file_path, file_format="sdf")
+        for molecule in molecules:
+            assert all(atom.stereochemistry is None for atom in molecule.atoms if atom.symbol == "N")
+
     # TODO: test_smiles_round_trip
 
     def test_smiles_add_H(self):

openff/toolkit/data/molecules/bromoaniline_planar_n.sdf

@@ -0,0 +1,52 @@
+bromoaniline_planar_n
+     RDKit          3D
+
+ 23 23  0  0  0  0  0  0  0  0999 V2000
+   -1.4458   -1.7176    0.0170 C   0  0  0  0  0  0  0  0  0  0  0  0
+   -0.8538   -0.4284    0.3054 N   0  0  0  0  0  0  0  0  0  0  0  0
+   -1.7577    0.6556    0.6347 C   0  0  0  0  0  0  0  0  0  0  0  0
+   -2.5959    1.0759   -0.5526 C   0  0  0  0  0  0  0  0  0  0  0  0
+    0.5492   -0.1859    0.2793 C   0  0  0  0  0  0  0  0  0  0  0  0
+    1.4468   -0.8382    1.0940 C   0  0  0  0  0  0  0  0  0  0  0  0
+    0.8233   -2.1401    2.3400 Br  0  0  0  0  0  0  0  0  0  0  0  0
+    2.7990   -0.5585    1.0200 C   0  0  0  0  0  0  0  0  0  0  0  0
+    3.2940    0.3726    0.1385 C   0  0  0  0  0  0  0  0  0  0  0  0
+    2.4042    1.0326   -0.6830 C   0  0  0  0  0  0  0  0  0  0  0  0
+    1.0591    0.7505   -0.6053 C   0  0  0  0  0  0  0  0  0  0  0  0
+   -0.8041   -2.2408   -0.7495 H   0  0  0  0  0  0  0  0  0  0  0  0
+   -1.3905   -2.3170    0.9344 H   0  0  0  0  0  0  0  0  0  0  0  0
+   -2.4538   -1.6308   -0.4308 H   0  0  0  0  0  0  0  0  0  0  0  0
+   -1.1854    1.5565    0.9356 H   0  0  0  0  0  0  0  0  0  0  0  0
+   -2.4752    0.4083    1.4382 H   0  0  0  0  0  0  0  0  0  0  0  0
+   -3.5063    0.4622   -0.6417 H   0  0  0  0  0  0  0  0  0  0  0  0
+   -1.9744    1.0650   -1.4963 H   0  0  0  0  0  0  0  0  0  0  0  0
+   -2.9400    2.1339   -0.4441 H   0  0  0  0  0  0  0  0  0  0  0  0
+    3.4618   -1.1096    1.6935 H   0  0  0  0  0  0  0  0  0  0  0  0
+    4.3565    0.5683    0.1054 H   0  0  0  0  0  0  0  0  0  0  0  0
+    2.7812    1.7746   -1.3875 H   0  0  0  0  0  0  0  0  0  0  0  0
+    0.4076    1.3108   -1.2862 H   0  0  0  0  0  0  0  0  0  0  0  0
+  1  2  1  0
+  2  3  1  0
+  3  4  1  0
+  2  5  1  0
+  5  6  1  0
+  6  7  1  0
+  6  8  2  0
+  8  9  1  0
+  9 10  2  0
+ 10 11  1  0
+ 11  5  2  0
+  1 12  1  0
+  1 13  1  0
+  1 14  1  0
+  3 15  1  0
+  3 16  1  0
+  4 17  1  0
+  4 18  1  0
+  4 19  1  0
+  8 20  1  0
+  9 21  1  0
+ 10 22  1  0
+ 11 23  1  0
+M  END
+$$$$