Conversation
…precate IBond.Stereo get/set and implement them in terms of Display. The IBond.Stereo is mainly based on MDL/CML model of bonds but then also mixes in an explicit E/Z setting. We now have correct ways of storing this (IDoubleBondStereochemistry). There is a slight behaviour change in that by default double bonds now return E_Z_BY_COORDS by default which mirrors the MDL semantics.
…ames Up/Down for convenience.
Note some functions had incorrect presumptions about what IBond.getStereo() returned and failed to check for UP + UP_INVERTED as is required. I have deprecated some very broken functions (in BondTools) where we have much better mechanism to handle this now. I have also left "JWM" comments where attention might be needed, in particular CML seems to get it wrong.
Both MOLfile and ChemDraw do this but for CDK's API we split them out and handle that when we do input/output.
…nup more usages of IBond.Stereo
|
Member
Author
|
Sorry for got to say if you look at the "C" it is mostly the DebugBond logic repeating log messages. 
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Some time ago now I added IBond.Display to replace for somewhat misnamed/abused
IBond.Stereo. However I left bond fields on the bond. Recent fixes to JChemPaint showed this was a bit of a pain to work with so this commit deprecates IBond.Stereo and moves the majority of the library over toIBond.Display.E_Z_BY_COORDINATESwhich is what you would get from a MOLFile. This is because it is implemented in terms ofgetDisplay(). The default forgetDisplay()isSolidfor both single/double bonds (unchanged).Display.CrossedANDDisplay.Wavy).There are quite a few places where
getStereo()was not used correctly. For example only Up/Down was being tested and not UpInverted/DownInverted. This is correctly ONLY if you came from a MOLfile. I have deprecated/marked these with a// JWMcomment indicating it perhaps needs some attention - in practise now withIStereoElementthe bond display really should only be touched in depiction since the stereochemistry is correctly captured elsewhere.Longer term, I plan to store
IStereoElementson the Atom/Bonds of a container for convenience and this is a step towards allowing that.