Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure correct stereo consistency (Fix #812) #813

Merged
merged 2 commits into from Feb 3, 2022
Merged

Ensure correct stereo consistency (Fix #812) #813

merged 2 commits into from Feb 3, 2022

Conversation

johnmay
Copy link
Member

@johnmay johnmay commented Feb 3, 2022

Fixes #812. This was quite a tricky one and very subtle. The minimal example is */C=C/C>> |$R$|, essentially due to AtomContainer2 AtomRef/BondRef the stereo gets in an inconsistent state. To handle these these implementations now remap stereo chemistry to their internal references when adding stereo.

A side-effect is addStereo() is now more robust and so some tests needed updating. There was also a slight miss-use of the connected components in SDG (my own doing) to split a single molecule. In this case we need to check if we split in the middle of stereochemistry. This also exposes another issue with Sgroups which I will open an issue for.

…s added to a molecule it's atom/bond components are remapped to the internal AtomRef/BondRef of the molecule in question. This ensure any future actions such as swapping and atom for another work correctly. This is now more robust and we get an error if we try to add stereochemistry that includes atom/bonds not in this molecule - this makes sense.
… function in the structure diagram generator. This

is a bit of a missue but reasonable - we now need check if the stereochemistry has not be split between two molecules.
We should also do the same of Sgroups (issue openned).
@sonarcloud
Copy link

sonarcloud bot commented Feb 3, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 13 Code Smells

0.0% 0.0% Coverage
27.2% 27.2% Duplication

@johnmay
Copy link
Member Author

johnmay commented Feb 3, 2022

Hmm I thought I configured the quality gate to only fail if there was major bugs

@egonw egonw merged commit ade938a into master Feb 3, 2022
@egonw
Copy link
Member

egonw commented Feb 3, 2022

I looked at the code, and always check the testing. Good to go.

@johnmay johnmay deleted the fix-812 branch February 9, 2022 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CDKToBeam NPE double bond stereo with labelled R-atoms
2 participants