#2739 - Wrong stereochemistry when reading cdxml reaction

[jblack-mestre]

Generic request

• PR name follows the pattern #1234 – issue name
• branch name does not contain '#'
• base branch (master or release/xx) is correct
• PR is linked with the issue
• task status changed to "Code review"
• code follows product standards
• regression tests updated

[AlexanderSavelyev]

could you please create at least one test (unit or regression) so that any new version does not break the change in the future
test can be added into unit tests here
https://github.com/epam/Indigo/blob/master/core/indigo-core/tests/tests/formats.cpp
or regression test. e.g.
https://github.com/epam/Indigo/blob/master/api/tests/integration/tests/formats/cdx.py

[jblack-mestre]

I figured out how to get the unit test working - let me know if this is ok or I need to change it

[AlexanderSavelyev]

thanks for the updated PR, but I think comparing two cdxml is not very stable. If Indigo changes any other piece of model - it will lead to different result. I would rather just calculate number of enhanced stereo
using stereocenters iterator for molecules .eg. this one
https://github.com/epam/Indigo/blob/master/core/indigo-core/molecule/src/molfile_saver.cpp#L802
or if it is hard at least save to extended SMILES
https://github.com/epam/Indigo/blob/master/core/indigo-core/tests/tests/formats.cpp#L157

[jblack-mestre]

In this case we need to count the different bond directions in the products. Both products have 1 OR stereocenter, but the first isomer should have a dashed wedge and the second should have a solid wedge at this stereocenter. Before the fix, both products had dashed wedges. Let me know if you think this is ok - if not, I can try the extended SMILES.

[AlexanderSavelyev]

yes, this looks much better. I started the CI and after successful run will merge

[jblack-mestre]

do you know why the address sanitizer it is failing with linux? I'm working on windows, so it is hard to see the problem

[AlexanderSavelyev]

yes, we see the issue. We will create a patch for you and just need to merge it into your branch

[AlexanderSavelyev]

Hi @jblack-mestre , could you please merge changes from this branch "fix_cdx_desctructors". After that the warning should be fixed

[jblack-mestre]

thanks, done

[AlexanderSavelyev]

all tests passed. I merged the PR. The change will be available in the next version (approx release date is next month)
@jblack-mestre Thanks so much for the fixes!