SourceForge has been redesigned. Learn more.
Close

#719 Fixing Add/Remove hydrogens

Needs_Review
closed
nobody
None
master
9
2014-01-19
2014-01-05
John May
No

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

Discussion

  • John May

    John May - 2014-01-05
    • 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

    John May - 2014-01-05

    Oh and adding hydrogens also updates the stereo elements.

     
  • John May

    John May - 2014-01-05
    • 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

    John May - 2014-01-06
    • 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

    John May - 2014-01-08
    • Priority: 1 --> 9
     
  • John May

    John May - 2014-01-12

    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

    John May - 2014-01-19

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

     
  • John May

    John May - 2014-01-19
    • status: open --> closed
     

Log in to post a comment.