#35 pharmacophore update

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

been updating the pharmcophore code a
bit. The primary change is to create a subclass of QueryAtomContainer
called PharmacophoreQuery that will allow us ot consider things such
as excluded volumes. The previous form of the code did not allow us to
do this.

While it does add a new class, code that uses the pharmacophore
classes only needs to be updated by changing QueryAtomContainer (or
IQueryAtomContainer) to PharmacophoreQuery. Everything else should
stay the same.

Other changes also include updating to JDK 1.5 idioms and addition of
tests and annotations.

This branch is based of the SF master, so I request a review and
subsequent merge into master. You can pull these updates from me by
doing

git pull git://rguha.ath.cx/cdk pcore

Discussion

  • Egon Willighagen

    Rajarshi asked me to put up the branch on a machine here in Uppsala, as his ath.cx machine is going down, so please check the patch from:

    git pull http://pele.farmbio.uu.se/git/rajarshi.git/ pcore

     
  • Egon Willighagen

    Ad f99d0a0def3013450104e35a3bf731d5fc5b79e3:

    appending one char can be done as char, e.g.

    '[' instead of "["

    Ad 7870121695663b1c64c9ad6505e551e037b63765:

    can you explain that change? Would an equals not be more precise?

    Ad de3ad6edba5979b8bd2e37a225b5b2689779631c:

    please fix this example code in one of the JavaDocs (missing bracket):

      • List<IQueryAtomContainer> defs = readPharmacophoreDefinitions"mydefs.xml");
      • List<PharmacophoreQuery> defs = readPharmacophoreDefinitions"mydefs.xml");

    Also, can't you just import PharmacophoreQuery so you do not need to have all those org.openscience.cdk.pharmacophore in the JavaDoc {@link }'s and @see's...? as in:

      • @return A list of {@link IQueryAtomContainer} objects
      • @return A list of {@link org.openscience.cdk.pharmacophore.PharmacophoreQuery} objects
     
  • Rajarshi Guha

    Rajarshi Guha - 2009-04-24

    Ad 7870121695663b1c64c9ad6505e551e037b63765:
    can you explain that change? Would an equals not be more precise?

    Since I pulled from pele, I don't have the whole history here. So I don't know which code you're referring to. Can you provide the diff for this query?

    Also, can't you just import PharmacophoreQuery so you do not need to have
    all those org.openscience.cdk.pharmacophore in the JavaDoc {@link }'s and
    @see's...? as in:

    I thought of that - but this happens when that class is not used anywhere in that source file. By forcing an import, we unnecessarily improt a class that will not be used. While I realzie that some other class will likely use the imported class, it seem ineffcieint to import classes that don't get used just to make documentation neater

    I've addressed the other concers - I'll wait to hear about the two issues above and then post a patch to be applied after the current changes have been applied

     
  • Rajarshi Guha

    Rajarshi Guha - 2009-04-27

    Addressed your comments (except the one about full package names in Javadocs - see my previous comment). The attached patch should be applied on top of the original patch

     
  • Egon Willighagen

    commit 7870121695663b1c64c9ad6505e551e037b63765
    Author: Rajarshi Guha rajarshi.guha@gmail.com
    Date: Thu Mar 12 15:39:57 2009 -0400

    Updated the toString tests
    

    diff --git a/src/test/org/openscience/cdk/pharmacophore/PharmacophoreQueryAngleBondTest.java b/src/test/org/openscience/cdk/p
    index a8f5b4b..5936fed 100644
    --- a/src/test/org/openscience/cdk/pharmacophore/PharmacophoreQueryAngleBondTest.java
    +++ b/src/test/org/openscience/cdk/pharmacophore/PharmacophoreQueryAngleBondTest.java
    @@ -83,6 +83,6 @@ public class PharmacophoreQueryAngleBondTest {
    PharmacophoreQueryAtom qatom3 = new PharmacophoreQueryAtom("blah", "C");
    PharmacophoreQueryAngleBond qbond1 = new PharmacophoreQueryAngleBond(qatom1, qatom2, qatom3, 54.735);
    String repr = qbond1.toString();
    - Assert.assertEquals("AC::Amine [[CX2]N]::aromatic [c1ccccc1]::blah [C]::[54.74 - 54.74]", repr);
    + Assert.assertEquals(0, repr.indexOf("AC::Amine [[CX2]N]::aromatic [c1ccccc1]::blah [C]::[54.74 - 54.74]"));
    }
    }
    \ No newline at end of file
    diff --git a/src/test/org/openscience/cdk/pharmacophore/PharmacophoreQueryBondTest.java b/src/test/org/openscience/cdk/pharma
    index c3baca3..74a72bf 100644
    --- a/src/test/org/openscience/cdk/pharmacophore/PharmacophoreQueryBondTest.java
    +++ b/src/test/org/openscience/cdk/pharmacophore/PharmacophoreQueryBondTest.java
    @@ -78,7 +78,7 @@ public class PharmacophoreQueryBondTest {
    PharmacophoreQueryAtom qatom1 = new PharmacophoreQueryAtom("Amine", "[CX2]N");
    PharmacophoreQueryAtom qatom2 = new PharmacophoreQueryAtom("aromatic", "c1ccccc1");
    PharmacophoreQueryBond qbond1 = new PharmacophoreQueryBond(qatom1, qatom2, 1.0, 2.0);
    - String repr = qbond1.toString();
    - Assert.assertEquals("DC::Amine [[CX2]N]::aromatic [c1ccccc1]::[1.0 - 2.0]", repr);
    + String repr = qbond1.toString();
    + Assert.assertEquals(0, repr.indexOf("DC::Amine [[CX2]N]::aromatic [c1ccccc1]::[1.0 - 2.0]"));
    }
    }

     
  • Egon Willighagen

    That's the patch I was refering too...

     
  • Rajarshi Guha

    Rajarshi Guha - 2009-04-27

    OK. The uploaded patch fixes issues raised in your comments (and should be applied after the original patch)

     
  • Egon Willighagen

    Applied to master.

     

Log in to post a comment.

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

Sign up for the SourceForge newsletter:





No, thanks