#35 pharmacophore update

[Rajarshi Guha]

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

[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");</iqueryatomcontainer>
• • List<pharmacophorequery> defs = readPharmacophoreDefinitions"mydefs.xml");</pharmacophorequery>

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]

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]

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

[Rajarshi Guha]

patch to address review comments

[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]

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

[Egon Willighagen]

Applied to master.