Showing posts with label CDK. Show all posts
Showing posts with label CDK. Show all posts

Monday, May 10, 2010

How to use GitHub for [CDK|Bioclipse] code review

Triggered by posts in the past three days, I though about writing up a short tutorial on how to perform code review for existing code on GitHub. Therefore, this applied to CDK and Bioclipse source code, many but will work for any project hosted in GitHub. Even if it is not, you could consider putting up a copy there yourself. This example will demonstrate the procedure on CDK functionality in Bioclipse in the bioclipse.cheminformatics repository.

Click on the images to get a higher resolution version.

Step 1: find the class you want to review
Use the GitHub web interface to browse your way towards the source code of the class you want to review. For example, the SmartsMatchingHelper.java:



Step 2: identify something you like to comment on
Next step is to perform some code reviewing. For example, we might want to ask something about how parseProperty() works:

Now, this page on GitHub does not provide the means to leave comments; instead, you comment on commits.

Step 3: find the last commit that touched the line you like to comment on
Git has a blame option (also called annotate) which will show you for each line who last changed that line. The GitHub web page makes this functionality available with the 'blame' link just above the first line of the source code:

This link will lead us to a page with a new column on the left side showing commit hashes, name of the commit author, and the first few characters of the commit message. For example, the web page bits relevant to code we want to comment on, looks like:

This shows us that commit 3ce78ba5 is the one we are interested in:



Step 4: Look up the line again and add a comment
In the web page with the appropriate commit looked in the previous step, you scroll down to the line you want to comment on. If you hover over that line, a blue comment bubble will show up on the left side:

Clicking that blue comment icon, you get a dialog where you can enter your comment:

The 'Add Line Note' button confirms and saves your comment:



Step 5: inform the commiter about your review
The next step would be to inform the commit author. GitHub actually helps here, and should send a message, like this one:

But it would certainly not hurt of you filed a bug report or sent an email.

Now, I should only convert this into a screencast...

Monday, February 1, 2010

Validating MDL SD files and Symyx molfiles with the CDK

Bioclipse 2.0 introduced a new, powerful molecular table support, and we have been eager to test that on large SD files. A recent ChEBI SD file failed to open, and eyes were immediately at the CDK, which is the cheminformatics library used in Bioclipse.

After careful investigations, it turned out that the ChEBI file contained a few entries which were not MDL molfiles, but queries for the ISISBase system. Those cannot be read by the CDK MDLV2000Reader. However, it crashed on it, instead of failing more savely. That's not nice, and fixed. But, the problem is rather recurrent, and the reason why I like CML so much: invalid input. CML, based on XML, has several general validation approaches that give in-depth error messages of what is wrong with the file.

So, I asked on the BOx what the Open Source cheminformatics community had to offer for this. Turns out that several tools find problems in the files, but none could report where the error occurred.

Validation
Now, some time ago, I played with two reading modes, RELAXED and STRICT, as faulty files is core cheminformatics material, and the software is blamed if the QSAR model resulting from it is not good (seriously). Anyways, a small API change in the CDK would make a validating MDLV2000Reader quite a step closer, but I had not followed up on it until last Friday where I patch I was reviewing caused 6 new unit test fails. The new fails were caused by a assumption which turned out the be false in the test files used in those 6 unit tests.

The MDL (or Symyx) molfile specifications (not an Open Specification) defines an atom block line as:
xxxxx.xxxxyyyyy.yyyyzzzzz.zzzz aaaddcccssshhhbbbvvvHHHrrriiimmmnnneee
but does not specify which fields are optional. And indeed, many tools around save MDL molfiles with one or more fields missing, leading to shorter than expected line lengths. And, as you might have expected, the failing unit tests had files with lines missing the field introduced by the patch, causing Exceptions being thrown around. I have yet to make up my mind of the lack of those fields is a problem in the file, or allowed by the format. In either case, the information from that field is not available, and the reader could safely ignore the missing information. Per user demand.

Now, personally, I rather send the file back to the user with a proper error report and show them what is wrong with the file. Or better, provide them with a MDL V2000 text editor (e.g. in Bioclipse) which would graphically highlight errors, as many of us are used to with Eclipse:


CDK Patch
So, I am hacking up a patch for CDK master to allow error reporting by IChemObjectReaders. The initial version of the API update and use in the MDLV2000Reader are available as Gist 290659. They are not final yet, as I realized when making the above screenshot, that merely int col is not enough, and that I actually need the startCol and endCol positions instead. Also, there are only an error level at this moment, and no warning level as in the screenshot.

That said, I created a jar (ant dist-large) and saved it as mdlCheck.jar, and wrote a bit of Groovy:

which defines a class implementing the new IChemObjectReaderErrorHandler and then reads a MDL molfile. And the output looks like it fulfills my needs:
$ CLASSPATH=mdlCheck.jar groovy mdlCheck.groovy src/test/data/mdl/test6.sdf 
location: 5, 35: Could not parse mass difference field.                                                                          
  -> For input string: ""                                                                                                        
location: 6, 35: Could not parse mass difference field.                                                                          
  -> For input string: ""                                                                                                        
Note to myself, that atom block does not like like a MDL molfile atom block at all! Every second line outputs the Exception passed to the error handler. I have to say, those messages are rather cryptic, but resulting from a NumberFormatException, if not mistaken.

Or, another common found issue (using D and T as element symbols):
$ CLASSPATH=mdlCheck.jar groovy mdlCheck.groovy src/test/data/mdl/hisotopes.mol
location: 6, 32: Invalid element type. Must be an existing element, or one in: A, Q, L, LP, *.
location: 7, 32: Invalid element type. Must be an existing element, or one in: A, Q, L, LP, *.

Enough for now... dinner time.

Saturday, August 29, 2009

Reminder: my talk in Frankfurt on Monday; Want to meet up?

Quick and short reminder about my Open Knowledge: Reproducibility in Cheminformatics with Open Data, Open Source and Open Standards talk on Monday. The session is great anyway, with other talks from Cameron, John and someone from Berlin on a Open Access HTS system (which reminds me to talk about the Open Access and that the term is tainted).

I still have a free program, other than I want to see Google Wave in action (and while I have receive my invitation, I have not received a login account yet). There is a potentially interesting talk about Second Generation Small Molecule Therapeutics by 15:00. But no plans otherwise for the afternoon and/or evening.

If you like to talk about CDK, Bioclipse and/or the Blue Obelisk movement. Or about my talk on Open Data, Open Standards and Open Source (ODOSOS) in chemoinformatics.

If you happen to be around the Frankfurt Westend campus. In building 4, I think, the Hörsaalzentrum, where the conference is. Please let me know if you like to meet up. I hope to be online :), but no promise on that... should work at a Uni location, not? Let's see... This is how to ping me, and don't worry about redundancy.

Email: egon.willighagen at gmail dot com
IRC: #cdk at irc.freenode.net
Twitter: egonwillighagen
Identica: chemblaics
Blog: just leave a reply to this message

Wednesday, April 15, 2009

Saturday, January 3, 2009

Scripting JChemPaint

Stefan, Gilleain, Arvid and I had a JChemPaint Developers Workshop in Uppsala, to sprint the development of JChemPaint3, for which Niels layed out the foundation already a long time ago.

Gilleain and Arvid merged their branches into a single code base, while Stefan worked on the Swing application and applet. The Bioclipse SWT-based widget is being developed for Bioclipse2.

The new design separates widget/graphics toolkit specifics from the chemical drawing and editing logic. Regarding the editing functionality, this basically comes down to have a semantically meaningful edit API. This allows us to convert both Swing and SWT mouse events into things like addAtom("C", atom), which would add a carbon to an already existing atom. However, without too much phantasy, it allows adding a scripting language. This is what I have been working on. Right now, the following API is available from the Bioclipse2 JavaScript console (via the jcp namespace, in random order):
  • ICDKMolecule jcp.getModel()
  • IAtom getClosestAtom(Point2d)
  • setModel(ICDKMolecule) (for really fancy things)
  • removeAtom(IAtom)
  • IBond getClosestBond(Point2d)
  • updateView() (all edit command issue this automatically)
  • addAtom(String,Point2d)
  • addAtom(String,IAtom) (which works out coordinates automatically)
  • Point2d newPoint2d(double,double)
  • updateImplicitHydrogenCounts()
  • moveTo(IAtom, Point2d)
  • setSymbol(IAtom,String)
  • setCharge(IAtom,int)
  • setMassNumber(IAtom,int)
  • addBond(IAtom,IAtom)
  • moveTo(IBond,Point2d)
  • setOrder(IBond,IBond.Order)
  • setWedgeType(IBond,int)
  • IBond.Order getOrder(int)
  • zap() (sort of sudo rm -Rf /*)
  • cleanup() (calculate 2D coordinates from scratch)
  • addRing(IAtom,int)
  • addPhenyl(IAtom)
This API (many more method will follow) is not really aimed at the end user, who will simply point and click. The goal of this scripting language is, at least at this moment, to test the underlying implementation using Bioclipse. Future applications, however, may include simple scripts which use some logic to convert the editor content. For example, replacing a t-butyl fragment into a pseudo atom "t-Bu". The key thing to remember, is that this will allow Bioclipse to have non-CDK-based programs act on the JChemPaint editor content (e.g. using getModel() and setModel(ICDKMolecule)). More on that later.

A simple script could look like: Or, as screenshot: