#274 SMSD Chem filter updates plus few updates

Needs_Revision
closed
nobody
master (162)
5
2012-10-08
2010-09-24
Asad
No

a) This patch deals with redesigning/refactoring of the chemfilters.
b) Changes in the bond energy calculation for "R" grp.
c) MCSPlus bug fix for size match.
d) VF lib improvised in terms of code layout and speed.

Discussion

  • Asad

    Asad - 2010-09-24

    Patches zipped

     
  • Egon Willighagen

    Asad,

    some observations:

    • the first patch removes a number of files, which cannot relate to the commit message
    • some patches have no content:

    -rw-r--r-- 1 egonw egonw 0 2010-09-24 22:53 0006-Missed-few-changes-hence-I-have-to-copy-and-paste-th.patch
    -rw-r--r-- 1 egonw egonw 0 2010-09-24 22:53 0009-removed-the-old-test-and-added-new-set-of-test-cases.patch

    • patch 0001, MoleculeSanityCheck:

    please use variables names of at least 3 characters, and provide some information why it is OK to ignore an exception:

    • } catch (Exception e) {
    • }

    • please do not sign-off your own patches

    • patch 0003 seems to create files without license/copyright:

    diff --git a/src/main/org/openscience/cdk/smsd/filters/BaseFilter.java b/src/main/org/openscience/cdk/smsd/filters/BaseFilter.java
    new file mode 100644
    index 0000000..5b9c7b2
    --- /dev/null
    +++ b/src/main/org/openscience/cdk/smsd/filters/BaseFilter.java
    @@ -0,0 +1,69 @@
    +package org.openscience.cdk.smsd.filters;

    • the constructor method of ChemicalFilters does not have JavaDoc
    • the ChemicalFilters.sortResultsByEnergies() method should describe when an exception is thrown:

      • @throws CDKException
    • the 0003 ChemicalFilter class contains this comment:

    // actually, never thrown, but in the interface

    Can you explain which API 'throws' a CDKException but never really does? Can you please file a bug report for that?

    • 0003 EnergyFilter has several methods without JavaDoc. For all @Override methods, you can use:

    /* {@inheritDoc} /

    or so... search the existing source code for examples...

    • the IChemicalFilter interface lacks JavaDoc
    • about patch 0004... if you fix bugs, please provide a matching unit test
    • patch 0008 StereoFilterTest does not seem to have a copyright/license header

    Asad, I cleaned up the patches a bit, so that files do not get removed and added, resulting in a new .zip file. Please consider using those as starting point for patch revisions.

     
  • Egon Willighagen

    Asad is working on a new patch.

     
  • gilleain maclean torrance

    Reopened with a new patchset. There are more of them because they are split up finer. As an overview, we have:

    1) Copywrite statements on labelling package
    2) The ChemFilter refactoring
    3) Better TimeOuts
    4) BondEnergies accepting the atom symbol "R", with test
    5) Fixes to MCSPlus
    6) Some renaming in BKKCKCF
    7) An alternative default MCS algorithm
    8) Fix for filtering by energies
    9) The J-MCS implementation

     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:

JavaScript is required for this form.





No, thanks