Fix #532 - smilesAtomOutputOrder incorrect #535
Conversation
Handle disconnected components correctly when generating the _smilesAtomOutputOrder property. This requires: (a) the mapping from the fragment molecules back to the original molecule (b) the fragment maps to be sorted at the same time as the fragment smiles (if canonical)
This makes things a little less clear (which is why I left it out originally) but it's more efficient. Now, rather than updating vfragsmi with the sorted fragment smiles, the final result smiles is generated directly.
|
I'm not sure it's worth doing, but in the case of non-canonical SMILES, you could avoid a vector copy if you generated the flattenedAtomOrdering directly from within the main loop over the fragment smiles (rather than populating allAtomOrdering). |
|
Thanks for this Noel. @NadineSchneider : would you mind taking a look at this and seeing if you can figure out what the problem is? |
|
I meant to ask you about writing tests. In your test code, did you mean to set atom 0 to oxygen? "m->getAtomWithIdx(0)->setAtomicNum(8);" |
|
oh my, that is a stupid copy and paste bug. |
|
Ok, the fix is on master, along with a couple of simple tests. |
Handle disconnected components correctly when generating the _smilesAtomOutputOrder property. This requires:
(a) the mapping from the fragment molecules back to the original molecule
(b) the fragment maps to be sorted at the same time as the fragment smiles (if canonical)