#719 Fixing Add/Remove hydrogens

[John May]

Been meaning to get round to this for a while. This patch updates hydrogen adding and removal in the AtomContainerManipulator. The removeHydrogens now considers stereo-elements and so '[C@]([H])(C)(CC)O' can be correctly converted to '[C@H](C)(CC)O'. Also the method now checks whether a hydrogen can be represented as implicit. This changes the behaviour of molecular hydrogen which now does not return an empty container.

May also want to never remove bridging hydrogens but there is already a separate method for that.

https://github.com/johnmay/cdk/compare/cdk:master...patch%2Fmanipulator-hydrogen-handling

[John May]

• Description has changed:

Diff:

--- old
+++ new
@@ -1,4 +1,4 @@
-Been meaning to get round to this for a while. This patch updates hydrogen adding and removal in the AtomContainerManipulator. The removeHydrogens not considers stereo-elements and so '[C@]([H])(C)(CC)O' can be correctly converted to '[C@H](C)(CC)O'. Also the method now checks whether a hydrogen can be represented as implicit. This changes the behaviour of molecular hydrogen which now does not return an empty container.
+Been meaning to get round to this for a while. This patch updates hydrogen adding and removal in the AtomContainerManipulator. The removeHydrogens not considers stereo-elements and so '\[C@\](\[H\])(C)(CC)O' can be correctly converted to '\[C@H\](C)(CC)O'. Also the method now checks whether a hydrogen can be represented as implicit. This changes the behaviour of molecular hydrogen which now does not return an empty container.

 May also want to never remove bridging hydrogens but there is already a separate method for that.

[John May]

Oh and adding hydrogens also updates the stereo elements.

[John May]

• Description has changed:

Diff:

--- old
+++ new
@@ -1,4 +1,4 @@
-Been meaning to get round to this for a while. This patch updates hydrogen adding and removal in the AtomContainerManipulator. The removeHydrogens not considers stereo-elements and so '\[C@\](\[H\])(C)(CC)O' can be correctly converted to '\[C@H\](C)(CC)O'. Also the method now checks whether a hydrogen can be represented as implicit. This changes the behaviour of molecular hydrogen which now does not return an empty container.
+Been meaning to get round to this for a while. This patch updates hydrogen adding and removal in the AtomContainerManipulator. The removeHydrogens now considers stereo-elements and so '\[C@\](\[H\])(C)(CC)O' can be correctly converted to '\[C@H\](C)(CC)O'. Also the method now checks whether a hydrogen can be represented as implicit. This changes the behaviour of molecular hydrogen which now does not return an empty container.

 May also want to never remove bridging hydrogens but there is already a separate method for that.

[John May]

• stereo elements are added with IAtomContainer.add(IAtomContainer) - the elements are now also stored as a Set. I couldn't bare to do 'contains()' on the list.
• friendly clones of IAtom atom = org.clone(); and IBond bond = org.clone();

[John May]

One this has been patch I need to rewrite the hydrogen removal completely. Testing on some data and it's terribly slow thanks to 'getConnectedBondsMap'. I'll wait for this to be patched first though.

[Egon Willighagen]

OK, looks good. But I do state that I personally find "original" better than "org", and "copy" better a variable name than "cpy"...

Applied and pushed.

[John May]

A subsequent patch has replaced the method meaning the 'cpy' is no longer used.