#28 factoring out of ioformats

Needs_Review
closed
None
5
2012-10-28
2009-01-03
No

The CDK IO format definitions and matchers are quite useful outside the scope of just the CDK library, for example, for the core of Bioclipse. The IChemFormatMatcher implementations indeed do not depend on 'interfaces' or 'core' modules.

Only complication is that TestMethod and TestClass are being used, as well as DataFeatures. I have moved these into a second new module 'annotation' as they deal with meta data.

The branch egonw-ioformats is derived from cdk1.2.x/ and contains the necessary updates:

  • three new modules: annotation, ioformats, test-ioformats
  • new .cdkdepends etc for these
  • @cdk.module updates
  • replaces a use of MathTools.isOdd() with a local copy (one liner)

This patch is ready for review by two senior reviewers, as I like to see it included in the CDK 1.2.0 release.

Attached is a PNG with the new module dependency graph.

Discussion

  • Egon Willighagen

    CDK module deps with 'annotation' and 'ioformats'.

     
  • Egon Willighagen

    The full changelog:

    commit 271122ed99c855d798810196ae86736df85223bc
    Author: egonw egonw@eb4e18e3-b210-0410-a6ab-dec725e4b171
    Date: Sun Jan 4 09:39:24 2009 +0000

    Added missing dependencies on the new ioformats module
    
    git-svn-id: https://cdk.svn.sourceforge.net/svnroot/cdk/cdk/branches/egonw-ioformats@13796 eb4e18e3-b210-0410-a6ab-dec725
    

    commit 585d12d4201982622dded11de2676861612ede0d
    Author: egonw egonw@eb4e18e3-b210-0410-a6ab-dec725e4b171
    Date: Sat Jan 3 14:08:21 2009 +0000

    Introduced a ioformats module (along with test-ioformats), which can now be used without pulling interfaces, core and the
    
    git-svn-id: https://cdk.svn.sourceforge.net/svnroot/cdk/cdk/branches/egonw-ioformats@13730 eb4e18e3-b210-0410-a6ab-dec725
    

    commit de702bb7c1422c070feb459353b46df45a617bec
    Author: egonw egonw@eb4e18e3-b210-0410-a6ab-dec725e4b171
    Date: Sat Jan 3 14:03:08 2009 +0000

    Moved to the annotation module, allowing use of it without pulling deps on interfaces and core too
    
    git-svn-id: https://cdk.svn.sourceforge.net/svnroot/cdk/cdk/branches/egonw-ioformats@13729 eb4e18e3-b210-0410-a6ab-dec725
    

    commit 02fefb7118b4a166de1a2fc8dabcc0b8ad8617c3
    Author: egonw egonw@eb4e18e3-b210-0410-a6ab-dec725e4b171
    Date: Sat Jan 3 14:02:37 2009 +0000

    Removed depedency on MathTools
    
    git-svn-id: https://cdk.svn.sourceforge.net/svnroot/cdk/cdk/branches/egonw-ioformats@13728 eb4e18e3-b210-0410-a6ab-dec725
    

    commit c7277db65b8ea6888c9f61335d698fc18f12afbb
    Author: egonw egonw@eb4e18e3-b210-0410-a6ab-dec725e4b171
    Date: Sat Jan 3 14:02:10 2009 +0000

    Moved the two annotation classes into a separate module
    
    git-svn-id: https://cdk.svn.sourceforge.net/svnroot/cdk/cdk/branches/egonw-ioformats@13727 eb4e18e3-b210-0410-a6ab-dec725
    

    commit 8a17b3a3313afe638fac99f397c4e513ee28fc6d
    Author: egonw egonw@eb4e18e3-b210-0410-a6ab-dec725e4b171
    Date: Sat Jan 3 09:08:09 2009 +0000

    Setting up a branch, to split up 'io' into 'io' and 'ioformats'.
    
    git-svn-id: https://cdk.svn.sourceforge.net/svnroot/cdk/cdk/branches/egonw-ioformats@13723 eb4e18e3-b210-0410-a6ab-dec725
    

    As done with Git. From the git-svn-id you can look up the SVN commit revision. For example, the revision of the last patch is 13727, while commit 13723 created the egonw-ioformats branch.

    To review an individual patch, use this link:

    http://cdk.svn.sourceforge.net/viewvc/cdk?view=rev&revision=13727

     
  • Rajarshi Guha

    Rajarshi Guha - 2009-01-04

    In HINFormat.java, I would inline the test for odd numbers into the if statement rather than making it a separate function. Also, it is a little grating that a module will consist of 2 files - but if ti simplifies dependencies that's good. Overall, it looks good to me and I sign off

     
  • Egon Willighagen

    Ack, on the HIN thingy. Will change that, and mention in the comment what the math is supposed to test.

    I agree on the three class (TestClass|-Method + DataFeatures) annotation module... I could not think of a nicer solution. Fact is, that the ioformats classes do use the annotation too. Putting them in interfaces seemed weirder. Maybe it will grow in the future, if we start using more annotation instead of JavaDoc... maybe for cdk.bug for not compiling 'buggy' code or so...

    Anyway, if anyone knows a prettier solution for those three annotation classes, please let me know.

     
  • Egon Willighagen

    Fixed the HIN one liner.

     
  • Stefan Kuhn

    Stefan Kuhn - 2009-01-19

    Looks good for me

     
  • Egon Willighagen

    Rajarshi, Stefan, thanx for reviewing!

     
  • Egon Willighagen

    Applied to cdk1.2.x.

     

Log in to post a comment.