#690 Keep aromatic flags on SMILES load.

Accepted
closed
nobody
None
master
1
2013-10-07
2013-10-05
John May
No

As discussed, assign bond orders but keeps aromatic flags. Fixed the tests I could the others are because the previous parser didn't set aromatic flags for bracket atoms. The atom typer doesn't handle these and causes some failures in a couple of places.

https://github.com/johnmay/cdk/compare/hotfix;beam-aromatic-flags

Edit: was late - meant to say there are bond orders + aromatic flags now

Related

Patches: #690

Discussion

  • What are bracket atoms? And what do you see go wrong with the typer?

     
  • John May
    John May
    2013-10-05

    Bracket atom
    [C@H]

    Aromatic bracket atom
    [c+H]1cccccc1

    The carbon cation above was previously loaded as aliphatic by the old code. Then the first beam patch gave back a Kekule structure which typed okay. Now it give back a Kekule structure with aromatic flags. There are a couple of cases in the unit tests that the atom typer doesn't recognise some aromatic atoms.

    I think one was a carbon cation and another one was a sulphur anion. Will double check when I get a computer. Simple fix is to remove the atom typing check from the SMILEs test but thought I'd post it with the fails first.

    Sent from my iPhone

    On 5 Oct 2013, at 07:03, "Egon Willighagen" egonw@users.sf.net wrote:

    What are bracket atoms? And what do you see go wrong with the typer?

    [patches:#690] Keep aromatic flags on SMILES load.

    Status: open
    Created: Sat Oct 05, 2013 12:20 AM UTC by John May
    Last Updated: Sat Oct 05, 2013 12:20 AM UTC
    Owner: nobody

    As discussed, assign bond orders but keeps bond orders. Fixed the tests I could the others are because the previous parser didn't set aromatic flags for bracket atoms. The atom typer doesn't handle these and causes some failures in a couple of places.

    https://github.com/johnmay/cdk/compare/hotfix;beam-aromatic-flags

    Sent from sourceforge.net because cdk-patches@lists.sourceforge.net is subscribed to http://sourceforge.net/p/cdk/patches/

    To unsubscribe from further messages, a project admin can change settings at http://sourceforge.net/p/cdk/admin/patches/options. Or, if this is a mailing list, you can unsubscribe from the mailing list.


    October Webinars: Code for Performance
    Free Intel webinars can help you accelerate application performance.
    Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from
    the latest Intel processors and coprocessors. See abstracts and register >
    http://pubads.g.doubleclick.net/gampad/clk?id=60134791&iu=/4140/ostg.clktrk


    Cdk-patches mailing list
    Cdk-patches@lists.sourceforge.net
    https://lists.sourceforge.net/lists/listinfo/cdk-patches

     

    Related

    Patches: #690

  • John May
    John May
    2013-10-05

    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -1,4 +1,6 @@
    -As discussed, assign bond orders but keeps bond orders. Fixed the tests I could the others are because the previous parser didn't set aromatic flags for bracket atoms. The atom typer doesn't handle these and causes some failures in a couple of places. 
    +As discussed, assign bond orders but keeps aromatic flags. Fixed the tests I could the others are because the previous parser didn't set aromatic flags for bracket atoms. The atom typer doesn't handle these and causes some failures in a couple of places. 
    
     https://github.com/johnmay/cdk/compare/hotfix;beam-aromatic-flags
    
    +
    +Edit: was late - meant to see there are bond orders + aromatic flags now
    
     
  • John May
    John May
    2013-10-05

    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -3,4 +3,4 @@
     https://github.com/johnmay/cdk/compare/hotfix;beam-aromatic-flags
    
    -Edit: was late - meant to see there are bond orders + aromatic flags now
    +Edit: was late - meant to say there are bond orders + aromatic flags now
    
     
  • John May
    John May
    2013-10-05

    There's also a couple of other fixes which worked out of the box with the kekule structure. You'll see in the commits. I do agree on keeping the aromatic flags though but it does have some quirks.

     
    • status: open --> closed
    • Group: Needs_Review --> Accepted
     
  • Applied and pushed.