pub_soft/rdkit
3
0
Fork 0
mirror of https://github.com/rdkit/rdkit.git synced 2026-06-03 21:44:30 +08:00
Code Issues Packages Projects Releases Wiki Activity

BondDir not cleared from bonds that aren't stereoactive (#6162)

Browse Source 
* backup

* update .gitignore

* passes tests

* all tests now pass

* update release notes

* cleanup

* changes in response to review
This commit is contained in:
Greg Landrum
2023-03-07 15:15:11 +01:00
committed by GitHub
parent 8d859469c5
commit 58b79c6f8e
9 changed files with 137 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
.gitignore vendored
View File

@@ -6,7 +6,7 @@
/Code/JavaWrappers/csharp_wrapper/swig_csharp
/Data/Fonts/ComicNeue-*
/Data/Fonts/OFL.txt
/vcpkg/*
#- Eclipse files
/.project
/.pydevproject

63
Code/GraphMol/Chirality.cpp
View File

@@ -1192,10 +1192,12 @@ void findAtomNeighborsHelper(const ROMol &mol, const Atom *atom,
PRECONDITION(refBond, "bad bond");
neighbors.clear();
for (const auto bond : mol.atomBonds(atom)) {
if (bond == refBond) {
continue;
}
Bond::BondDir dir = bond->getBondDir();
if ((bond->getBondType() == Bond::SINGLE ||
(includeAromatic && bond->getBondType() == Bond::AROMATIC)) &&
bond->getIdx() != refBond->getIdx()) {
if (bond->getBondType() == Bond::SINGLE ||
(includeAromatic && bond->getBondType() == Bond::AROMATIC)) {
if (checkDir) {
if ((dir != Bond::ENDDOWNRIGHT) && (dir != Bond::ENDUPRIGHT)) {
continue;
@@ -1579,8 +1581,9 @@ std::pair<bool, bool> assignBondStereoCodes(ROMol &mol, UINT_VECT &ranks) {
boost::dynamic_bitset<> bondsToClear(mol.getNumBonds());
// find the double bonds:
for (auto dblBond : mol.bonds()) {
if (dblBond->getBondType() == Bond::DOUBLE) {
if (dblBond->getStereo() != Bond::STEREONONE) {
if (dblBond->getBondType() == Bond::BondType::DOUBLE) {
if (dblBond->getStereo() != Bond::BondStereo::STEREONONE &&
dblBond->getStereo() != Bond::BondStereo::STEREOANY) {
continue;
}
if (!ranks.size()) {
@@ -1601,7 +1604,7 @@ std::pair<bool, bool> assignBondStereoCodes(ROMol &mol, UINT_VECT &ranks) {
// look around each atom and see if it has at least one bond with
// direction marked:
// the pairs here are: atomrank,bonddir
// the pairs here are: atomIdx,bonddir
Chirality::INT_PAIR_VECT begAtomNeighbors, endAtomNeighbors;
bool hasExplicitUnknownStereo = false;
int bgn_stereo = false, end_stereo = false;
@@ -2052,6 +2055,8 @@ void legacyStereoPerception(ROMol &mol, bool cleanIt,
} else if (bond->getBondType() == Bond::DOUBLE) {
if (bond->getBondDir() == Bond::EITHERDOUBLE) {
bond->setStereo(Bond::STEREOANY);
bond->getStereoAtoms().clear();
bond->setBondDir(Bond::NONE);
} else if (bond->getStereo() != Bond::STEREOANY) {
bond->setStereo(Bond::STEREONONE);
bond->getStereoAtoms().clear();
@@ -2168,9 +2173,10 @@ void legacyStereoPerception(ROMol &mol, bool cleanIt,
bond->getStereo() == Bond::STEREONONE)) {
std::vector<Atom *> batoms = {bond->getBeginAtom(), bond->getEndAtom()};
for (auto batom : batoms) {
for (const auto &nbri :
boost::make_iterator_range(mol.getAtomBonds(batom))) {
auto nbrBndI = mol[nbri];
for (const auto nbrBndI : mol.atomBonds(batom)) {
if (nbrBndI == bond) {
continue;
}
if ((nbrBndI->getBondDir() == Bond::ENDDOWNRIGHT ||
nbrBndI->getBondDir() == Bond::ENDUPRIGHT) &&
(nbrBndI->getBondType() == Bond::SINGLE ||
@@ -2178,9 +2184,8 @@ void legacyStereoPerception(ROMol &mol, bool cleanIt,
// direction is set, and we know it's not because of our
// bond. What about other neighbors?
bool okToClear = true;
for (const auto &nbrj : boost::make_iterator_range(
mol.getAtomBonds(nbrBndI->getOtherAtom(batom)))) {
auto nbrBndJ = mol[nbrj];
for (const auto nbrBndJ :
mol.atomBonds(nbrBndI->getOtherAtom(batom))) {
if (nbrBndJ->getBondType() == Bond::DOUBLE &&
nbrBndJ->getStereo() != Bond::STEREOANY &&
nbrBndJ->getStereo() != Bond::STEREONONE) {
@@ -2200,10 +2205,13 @@ void legacyStereoPerception(ROMol &mol, bool cleanIt,
}
}
void updateDoubleBondStereo(ROMol &mol, const std::vector<StereoInfo> &sinfo) {
void updateDoubleBondStereo(ROMol &mol, const std::vector<StereoInfo> &sinfo,
bool cleanIt) {
boost::dynamic_bitset<> bondsTouched(mol.getNumBonds(), 0);
for (const auto &si : sinfo) {
if (si.type == Chirality::StereoType::Bond_Double) {
auto bond = mol.getBondWithIdx(si.centeredOn);
bondsTouched.set(bond->getIdx());
bond->setStereo(Bond::BondStereo::STEREONONE);
if (si.specified == Chirality::StereoSpecified::Specified) {
TEST_ASSERT(si.controllingAtoms.size() == 4);
@@ -2221,11 +2229,25 @@ void updateDoubleBondStereo(ROMol &mol, const std::vector<StereoInfo> &sinfo) {
}
} else if (si.specified == Chirality::StereoSpecified::Unknown) {
bond->setStereo(Bond::BondStereo::STEREOANY);
bond->getStereoAtoms().clear();
bond->setBondDir(Bond::BondDir::NONE);
} else if (si.specified == Chirality::StereoSpecified::Unspecified) {
assignBondCisTrans(mol, si);
}
}
}
if (cleanIt) {
for (auto bond : mol.bonds()) {
if (bondsTouched[bond->getIdx()] ||
bond->getBondType() != Bond::BondType::DOUBLE) {
continue;
}
// we didn't see it above, so it can't have stereo:
bond->setStereo(Bond::BondStereo::STEREONONE);
bond->setBondDir(Bond::BondDir::NONE);
bond->getStereoAtoms().clear();
}
}
}
void stereoPerception(ROMol &mol, bool cleanIt,
bool flagPossibleStereoCenters) {
@@ -2234,8 +2256,14 @@ void stereoPerception(ROMol &mol, bool cleanIt,
atom->clearProp(common_properties::_CIPCode);
atom->clearProp(common_properties::_ChiralityPossible);
}
for (auto bond : mol.bonds()) {
if (bond->getBondDir() == Bond::BondDir::EITHERDOUBLE) {
bond->setStereo(Bond::BondStereo::STEREOANY);
bond->getStereoAtoms().clear();
bond->setBondDir(Bond::BondDir::NONE);
}
}
}
// we need cis/trans markers on the double bonds... set those now:
MolOps::setBondStereoFromDirections(mol);
@@ -2255,7 +2283,7 @@ void stereoPerception(ROMol &mol, bool cleanIt,
}
}
// populate double bond stereo info:
updateDoubleBondStereo(mol, sinfo);
updateDoubleBondStereo(mol, sinfo, cleanIt);
if (cleanIt) {
Chirality::cleanupStereoGroups(mol);
}
@@ -3168,10 +3196,11 @@ void removeStereochemistry(ROMol &mol) {
}
for (auto bond : mol.bonds()) {
if (bond->getBondType() == Bond::DOUBLE) {
bond->setStereo(Bond::STEREONONE);
bond->setStereo(Bond::BondStereo::STEREONONE);
bond->getStereoAtoms().clear();
bond->setBondDir(Bond::BondDir::NONE);
} else if (bond->getBondType() == Bond::SINGLE) {
bond->setBondDir(Bond::NONE);
bond->setBondDir(Bond::BondDir::NONE);
}
}
std::vector<StereoGroup> sgs;

4
Code/GraphMol/FileParsers/cdxml_parser_catch.cpp
View File

@@ -721,8 +721,8 @@ TEST_CASE("CDXML") {
int i = 0;
for (auto &mol : mols) {
if (i == 0) {
CHECK(mol->getBondWithIdx(11)->getBondDir() ==
Bond::BondDir::EITHERDOUBLE);
CHECK(mol->getBondWithIdx(11)->getStereo() ==
Bond::BondStereo::STEREOANY);
}
CHECK(MolToSmiles(*mol) == expected[i++]);
}

2
Code/GraphMol/FileParsers/file_parsers_catch.cpp
View File

@@ -3578,7 +3578,7 @@ TEST_CASE("double bond stereo should not be set when the coords are all zero") {
M END)CTAB"_ctab;
REQUIRE(m);
REQUIRE(m->getBondBetweenAtoms(1, 2));
CHECK(m->getBondBetweenAtoms(1, 2)->getBondDir() == Bond::EITHERDOUBLE);
CHECK(m->getBondBetweenAtoms(1, 2)->getStereo() == Bond::STEREOANY);
}
TEST_CASE("Handle MRV_COORDINATE_BOND_TYPE data Substance Groups") {

4
Code/GraphMol/FileParsers/test1.cpp
View File

@@ -954,7 +954,6 @@ void testDblBondStereochem() {
m1 = MolFileToMol(fName);
TEST_ASSERT(m1);
TEST_ASSERT(m1->getBondWithIdx(0)->getStereo() == Bond::STEREOANY);
TEST_ASSERT(m1->getBondWithIdx(0)->getBondDir() == Bond::EITHERDOUBLE);
delete m1;
}
@@ -3180,8 +3179,7 @@ void testIssue3375684() {
RWMol *m = MolFileToMol(fName);
TEST_ASSERT(m->getBondBetweenAtoms(6, 7)->getBondType() == Bond::DOUBLE);
TEST_ASSERT(m->getBondBetweenAtoms(6, 7)->getBondDir() ==
Bond::EITHERDOUBLE);
TEST_ASSERT(m->getBondBetweenAtoms(6, 7)->getStereo() == Bond::STEREOANY);
delete m;
}
{

14
Code/GraphMol/FindStereo.cpp
View File

@@ -840,10 +840,10 @@ bool updateBonds(ROMol &mol, const std::vector<unsigned int> &aranks,
return needAnotherRound;
}
void cleanMolStereo(ROMol &mol, boost::dynamic_bitset<> &fixedAtoms,
boost::dynamic_bitset<> &knownAtoms,
boost::dynamic_bitset<> &fixedBonds,
boost::dynamic_bitset<> &knownBonds) {
void cleanMolStereo(ROMol &mol, const boost::dynamic_bitset<> &fixedAtoms,
const boost::dynamic_bitset<> &knownAtoms,
const boost::dynamic_bitset<> &fixedBonds,
const boost::dynamic_bitset<> &knownBonds) {
for (auto i = 0u; i < mol.getNumAtoms(); ++i) {
if (!fixedAtoms[i] && knownAtoms[i]) {
switch (mol.getAtomWithIdx(i)->getChiralTag()) {
@@ -865,9 +865,11 @@ void cleanMolStereo(ROMol &mol, boost::dynamic_bitset<> &fixedAtoms,
}
}
for (auto i = 0u; i < mol.getNumBonds(); ++i) {
auto bond = mol.getBondWithIdx(i);
if (!fixedBonds[i] && knownBonds[i]) {
// FIX only does known double bonds
mol.getBondWithIdx(i)->setStereo(Bond::BondStereo::STEREONONE);
bond->setStereo(Bond::BondStereo::STEREONONE);
bond->setBondDir(Bond::BondDir::NONE);
bond->getStereoAtoms().clear();
}
}
}

77
Code/GraphMol/catch_chirality.cpp
View File

@@ -3306,4 +3306,81 @@ TEST_CASE(
CHECK(sg.centeredOn != 15);
}
}
}
TEST_CASE(
"assignStereochemistry should clear crossed double bonds that can't have stereo") {
SECTION("basics") {
auto m = "CC=C(C)C"_smiles;
REQUIRE(m);
m->getBondWithIdx(1)->setBondDir(Bond::BondDir::EITHERDOUBLE);
bool clean = true;
bool flag = true;
bool force = true;
{
UseLegacyStereoPerceptionFixture reset_stereo_perception;
Chirality::setUseLegacyStereoPerception(false);
auto cp(*m);
RDKit::MolOps::assignStereochemistry(cp, clean, force, flag);
CHECK(cp.getBondWithIdx(1)->getBondDir() == Bond::BondDir::NONE);
}
{
UseLegacyStereoPerceptionFixture reset_stereo_perception;
Chirality::setUseLegacyStereoPerception(true);
auto cp(*m);
RDKit::MolOps::assignStereochemistry(cp, clean, force, flag);
CHECK(cp.getBondWithIdx(1)->getBondDir() == Bond::BondDir::NONE);
}
}
SECTION("make sure we don't mess with actual potential stereosystems") {
auto m = "CC=C(C)[13CH3]"_smiles;
REQUIRE(m);
m->getBondWithIdx(1)->setBondDir(Bond::BondDir::EITHERDOUBLE);
bool clean = true;
bool flag = true;
bool force = true;
{
UseLegacyStereoPerceptionFixture reset_stereo_perception;
Chirality::setUseLegacyStereoPerception(false);
auto cp(*m);
RDKit::MolOps::assignStereochemistry(cp, clean, force, flag);
// the crossed bond dir has been translated to unknown stereo:
CHECK(cp.getBondWithIdx(1)->getBondDir() == Bond::BondDir::NONE);
CHECK(cp.getBondWithIdx(1)->getStereo() == Bond::BondStereo::STEREOANY);
}
{
UseLegacyStereoPerceptionFixture reset_stereo_perception;
Chirality::setUseLegacyStereoPerception(true);
auto cp(*m);
RDKit::MolOps::assignStereochemistry(cp, clean, force, flag);
// the crossed bond dir has been translated to unknown stereo:
CHECK(cp.getBondWithIdx(1)->getBondDir() == Bond::BondDir::NONE);
CHECK(cp.getBondWithIdx(1)->getStereo() == Bond::BondStereo::STEREOANY);
}
}
SECTION("make sure stereoatoms are also cleared") {
auto m = "CC=C(C)C"_smiles;
REQUIRE(m);
m->getBondWithIdx(1)->setBondDir(Bond::BondDir::EITHERDOUBLE);
m->getBondWithIdx(1)->setStereoAtoms(0, 3);
bool clean = true;
bool flag = true;
bool force = true;
{
UseLegacyStereoPerceptionFixture reset_stereo_perception;
Chirality::setUseLegacyStereoPerception(false);
auto cp(*m);
RDKit::MolOps::assignStereochemistry(cp, clean, force, flag);
CHECK(cp.getBondWithIdx(1)->getBondDir() == Bond::BondDir::NONE);
CHECK(cp.getBondWithIdx(1)->getStereoAtoms().empty());
}
{
UseLegacyStereoPerceptionFixture reset_stereo_perception;
Chirality::setUseLegacyStereoPerception(true);
auto cp(*m);
RDKit::MolOps::assignStereochemistry(cp, clean, force, flag);
CHECK(cp.getBondWithIdx(1)->getBondDir() == Bond::BondDir::NONE);
CHECK(cp.getBondWithIdx(1)->getStereoAtoms().empty());
}
}
}

2
Code/GraphMol/molopstest.cpp
View File

@@ -7732,7 +7732,6 @@ void testGithub1614() {
{
RWMol nm(*m);
MolOps::setDoubleBondNeighborDirections(nm);
// nm.debugMol(std::cerr);
bool force = true, cleanIt = true;
MolOps::assignStereochemistry(nm, cleanIt, force);
TEST_ASSERT(nm.getBondBetweenAtoms(4, 5)->getStereo() == Bond::STEREOE);
@@ -7754,7 +7753,6 @@ void testGithub1614() {
{
RWMol nm(*m);
MolOps::setDoubleBondNeighborDirections(nm);
// nm.debugMol(std::cerr);
bool force = true, cleanIt = true;
MolOps::assignStereochemistry(nm, cleanIt, force);
TEST_ASSERT(nm.getBondBetweenAtoms(4, 5)->getStereo() == Bond::STEREOE);

1
ReleaseNotes.md
View File

@@ -10,6 +10,7 @@
- The ring-finding functions will now run even if the molecule already has ring information. Older versions of the RDKit would return whatever ring information was present, even if it had been generated using a different algorithm.
- The canonical SMILES and CXSMILES generated for molecules with enhanced stereochemistry (stereo groups) is different than in previous releases. The enhanced stereochemistry information and the stereo groups themselves are now canonical. This does *not* affect molecules which do not have enhanced stereo and will not have any effect if you generate non-isomeric SMILES. This change also affects the output of the MolHash and RegistrationHash code when applied to molecules with enhanced stereo.
- The doIsomericSmiles parameter in Java and C# ROMol.MolToSmiles() now defaults to true (previously it was false), thus aligning to the C++ and Python behavior.
- Double bonds which are marked as crossed (i.e. `bond.GetBondDir() == Bond.BondDir.EITHERDOUBLE`) now have their BondStereo set to `Bond.BondStereo.STEREOANY` and the BondDir information removed by default when molecules are parsed or `AssignStereochemistry()` is called with the `cleanIt` argument set to True.
## Bug Fixes: