Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#35 pharmacophore update

Needs_Review
closed
None
5
2012-10-28
2009-03-13
Rajarshi Guha
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

  • 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

     
  • 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

     
  • 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]"));
    }
    }

     
  • 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)

     
  • Applied to master.