bugfix stereochemistry of conjugated bonds #40
Conversation
|
Thanks! @ljn917 are you able to review or should I? |
|
@connorcoley Sure, I will do it. |
|
@hesther @connorcoley Sincere apologies for my very late response. I did a simple refactor to remove the My code is below because I don't have write access to Esther's branch. |
|
Just ran a few tests and seems fine to me! Thanks! I can either overwrite the commit here, or you can make a new PR from your fork, whatever works best for you! |
|
I think it is better to add my commits here, because we can track
everything in the same PR.
…On Thu, Aug 24, 2023, 5:05 PM hesther ***@***.***> wrote:
Just ran a few tests and seems fine to me! Thanks! I can either overwrite
the commit here, or you can make a new PR from your fork, whatever works
best for you!
—
Reply to this email directly, view it on GitHub
<#40 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AALDTHU36EWPDVXKMMJBNLDXW4KN3ANCNFSM6AAAAAAUPJRIK4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
hesther
force-pushed
the
debug_conjugated
branch
from
d417129 to
979fc0d
Compare
|
Okay I copy/pasted the three changed files (bonds.py, main.py and test_rdchiral_cases.json), squashed the two commits and force-pushed to have a single commit |
Hi, this is the promised bugfix for #39. The problem was that depending on the atom numbering the direction (up/down) of a double bond system might change for the whole molecule. If that happens, and the template specifies the beginning of a conjugated system but not all of it, one needs to reverse all directions that were copied from the reactants and not set by the template. Jiannan's test script in #39 should be sufficient to test, and I also ran this on a database of 15k reactions to check whether I introduced any unwanted problems. Ran fine, but I saw that there are other cases also for tetrahedral chirality where a similar problem occurs (wrong outcome or wrong template depending on atom numbering). Might work on that soon if I find some spare time. Anyways, this PR fixes all problems concerning wrong cis/trans reaction outcomes, and should be ready to merge.