From 4f0521e1163cd62a19f19bd2b624ef5427a615a8 Mon Sep 17 00:00:00 2001 From: jfkonecn Date: Wed, 2 Jun 2021 23:13:13 -0400 Subject: [PATCH] fixes #3967 (#4190) * fixes #3967 * modernize test Co-authored-by: greg landrum --- .gitignore | 2 ++ Code/GraphMol/Canon.cpp | 26 ++++++++++++++++++++++++++ Code/GraphMol/SmilesParse/test.cpp | 22 ++++++++++++++++++++++ 3 files changed, 50 insertions(+) diff --git a/.gitignore b/.gitignore index c4bcb9015..3c1ac5625 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,8 @@ #- Eclipse files /.project /.pydevproject +/.cproject +/.settings/ #- CLion files /cmake-build-* diff --git a/Code/GraphMol/Canon.cpp b/Code/GraphMol/Canon.cpp index 542a563e1..6d83b2054 100644 --- a/Code/GraphMol/Canon.cpp +++ b/Code/GraphMol/Canon.cpp @@ -350,10 +350,25 @@ void canonicalizeDoubleBond(Bond *dblBond, UINT_VECT &bondVisitOrders, // If we're not looking at the bonds used to determine the // stereochemistry, we need to flip the setting on the other bond: const INT_VECT &stereoAtoms = dblBond->getStereoAtoms(); + + auto isClosingRingBond = [](Bond *bond) { + if (bond == nullptr) { + return false; + } + auto beginIdx = bond->getBeginAtomIdx(); + auto endIdx = bond->getEndAtomIdx(); + return beginIdx > endIdx && + beginIdx - endIdx > 1 && + bond->hasProp(common_properties::_TraversalRingClosureBond); + }; + + auto isFlipped = false; + if (atom1->getDegree() == 3 && std::find(stereoAtoms.begin(), stereoAtoms.end(), static_cast(atom1ControllingBond->getOtherAtomIdx( atom1->getIdx()))) == stereoAtoms.end()) { + isFlipped = true; atom2Dir = (atom2Dir == Bond::ENDUPRIGHT) ? Bond::ENDDOWNRIGHT : Bond::ENDUPRIGHT; } @@ -363,6 +378,17 @@ void canonicalizeDoubleBond(Bond *dblBond, UINT_VECT &bondVisitOrders, std::find(stereoAtoms.begin(), stereoAtoms.end(), static_cast(firstFromAtom2->getOtherAtomIdx( atom2->getIdx()))) == stereoAtoms.end()) { + isFlipped = true; + atom2Dir = (atom2Dir == Bond::ENDUPRIGHT) ? Bond::ENDDOWNRIGHT + : Bond::ENDUPRIGHT; + } + + if (!isFlipped && + (isClosingRingBond(dblBond) || + (isClosingRingBond(secondFromAtom1) && + !secondFromAtom1->getIsAromatic() && + secondFromAtom1->getBondDir() != Bond::NONE)) + ) { atom2Dir = (atom2Dir == Bond::ENDUPRIGHT) ? Bond::ENDDOWNRIGHT : Bond::ENDUPRIGHT; } diff --git a/Code/GraphMol/SmilesParse/test.cpp b/Code/GraphMol/SmilesParse/test.cpp index 4a69cda45..4e272cb66 100644 --- a/Code/GraphMol/SmilesParse/test.cpp +++ b/Code/GraphMol/SmilesParse/test.cpp @@ -4339,6 +4339,27 @@ void testOSSFuzzFailures() { BOOST_LOG(rdInfoLog) << "done" << std::endl; } +void testGithub3967() { + BOOST_LOG(rdInfoLog) << "-------------------------------------" << std::endl; + BOOST_LOG(rdInfoLog) << "Testing Github Issue 3967: Double bond stereo gets " + "flipped by SMILES reader/writer" + << std::endl; + + { + auto mol = "C=c1s/c2n(c1=O)CCCCCCC\\N=2"_smiles; + TEST_ASSERT(mol); + auto smi = MolToSmiles(*mol); + TEST_ASSERT(smi == "C=c1s/c2n(c1=O)CCCCCCC\\N=2"); + } + { + auto mol = "C1=C\\C/C=C2C3=C/C/C=C\\C=C/C\\3C\\2\\C=C/1"_smiles; + TEST_ASSERT(mol); + auto smi = MolToSmiles(*mol); + TEST_ASSERT(smi == "C1=C\\C/C=C2C3=C/C/C=C\\C=C/C\\3C\\2\\C=C/1"); + } + BOOST_LOG(rdInfoLog) << "\tdone" << std::endl; +} + int main(int argc, char *argv[]) { (void)argc; (void)argv; @@ -4417,6 +4438,7 @@ int main(int argc, char *argv[]) { testdoRandomSmileGeneration(); testGithub1028(); testGithub3139(); + testGithub3967(); #endif testOSSFuzzFailures(); }