[Cdk-devel] [ cdk-Patches-3474944 ] proper handling of pseudoatoms in MDL files

[SourceForge.net <no...@so...>]

Patches item #3474944, was opened at 2012-01-17 06:16
Message generated for change (Comment added) made by egonw
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3474944&group_id=20024

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: cdk-1.4.x
Group: Needs Revision
Status: Open
Resolution: None
Priority: 6
Private: No
Submitted By: Rajarshi Guha (rajarshi)
Assigned to: Rajarshi Guha (rajarshi)
Summary: proper handling of pseudoatoms in MDL files

Initial Comment:
I was looking at a bug report where reading an SD file containing pseudoatoms (such as Gln, Ile etc) worked fine, but writing it out crashed with an exception.

The reason for this is that during reading the atom with label Gln, is read in as a PseudoAtom with label "Gln" but symbol "R".

The MDLV2000Writer sees the symbol R for this atom and assumes that it is an R group and then tries to parse the label to get an R-group number. Obviously for this case it fails.

The problem is that *all* PseudoAtom objects get a symbol of R. While generally OK, this is not always correct for sdf/mol files. There are two possible solutions:

1. Modify PseudoAtom and related classes to allow one to specify the symbol as well as the label
2. Modify MDLV2000Reader & MDLV3000Reader to set the symbol for a pseudo atom explicitly to the label.

This patch implements 2

----------------------------------------------------------------------

>Comment By: Egon Willighagen (egonw)
Date: 2012-01-22 02:45

Message:
Rajarshi, if you agree with application to cdk-1.4.x, please find attached
your patch signed off by me, and a patch with fixes, which you need to
review.

If you prefer master only, then let me know, and I'll push your patch to
master.

----------------------------------------------------------------------

Comment By: Egon Willighagen (egonw)
Date: 2012-01-22 01:29

Message:
The patch now only applies to master... should we not try to apply this too
cdk-1.4.x?

----------------------------------------------------------------------

Comment By: Rajarshi Guha (rajarshi)
Date: 2012-01-21 10:39

Message:
Added a new patch that addresses all comments - ignore the initial patch. I
can't seem to remove the first patch

----------------------------------------------------------------------

Comment By: Egon Willighagen (egonw)
Date: 2012-01-21 02:59

Message:
Rajarshi, my comments...

1. can you please add a test for the writer too, showing that something is
fixed, like that crash?

2. Assert.assertTrue(molecule.getAtom(4).getSymbol().equals("Gln"));

should be: 

Assert.assertEquals("Gln", molecule.getAtom(4).getSymbol());

there is a second assertTrue that needs a similar replacement.

3. add the same unit test for the MDLV3000Reader too, please

Egon

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3474944&group_id=20024