No coords atropisomers - fix smiles output of atrop wedges after reordering #7418
Conversation
clang-format
document representation
| "ZM374979_atrop2.sdf"}; | ||
|
|
||
| for (auto file : files) { | ||
| std::cout << "File: " << file << std::endl; |
There was a problem hiding this comment.
Please don't include output like this in the tests, it makes the running the tests really noisy and doesn't add anything. If there's an informative message that you want to output when a test fails, you can do that with catch2's INFO() macro.
I know that I approved several older PRs that also have a large amount of output, but I plan to do a PR to clean that up in the not-too-distant future.
There was a problem hiding this comment.
You fixed that one example, but there are a number of other outputs to std::cout in that file and in the other tests added for this PR
There was a problem hiding this comment.
I think I got them all - probably one I didn't put in.
| Atropisomers::AtropAtomAndBondVec atomAndBondVecs[2]; | ||
| if (!Atropisomers::getAtropisomerAtomsAndBonds( | ||
| bondNbr, atomAndBondVecs, mol)) { | ||
| throw RDKit::SmilesParseException( |
There was a problem hiding this comment.
This shouldn't be a SmilesParseException since we aren't parsing SMILES at this point.
This should either be a ValueError if it's something which could happen but shouldn't or handled using the RDKit's CHECK_INVARIANT() if this only happens in the case of an RDKit bug.
There was a problem hiding this comment.
changed to value error
|
Thanks @tadhurst-cdd ! |
Reference Issue
Needed for several things, including ("the Canonicalization of Enhanced stereo for](#7041")
What does this implement/fix? Explain your changes.
Allows atropisomers to be represented with wedge bonds even when there are no coordinates. The assumption is that the lowest numbered "other atom" are each end are considered to be on the same side of the atropisomer bond.
Any other comments?