Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Patch/sgrouplayout #173

Merged
merged 16 commits into from Nov 22, 2015
Merged

Patch/sgrouplayout #173

merged 16 commits into from Nov 22, 2015

Conversation

johnmay
Copy link
Member

@johnmay johnmay commented Nov 10, 2015

I think probably ready for a release after this patch. This patch primarily focusses on layout of Sgroup brackets, positional variation, and attachment points.

As noted by the IUPAC guide attachment points should be drawn as crossing wavy line.

image image

Testable example:
image

SMILES: c1ccccc1.C*.C* |$;;;;;;;_AP1;;R5$,m:6:0.1.2.3.4.5,m:8:0.1.2.3.4.5|
Full Code:

IAtomContainer mol = new SmilesParser(SilentChemObjectBuilder.getInstance()).parseSmiles("c1ccccc1.CC.C[R5]");

// set attach point
AtomContainerManipulator.replaceAtomByAtom(mol, mol.getAtom(7), new PseudoAtom("_AP1"){{
    setAttachPointNum(1);
}});

// make positional variation
Sgroup pv1 = new Sgroup();
pv1.setType(SgroupType.ExtMulticenter);
pv1.addAtom(mol.getAtom(6));
pv1.addBond(mol.getBond(6));
pv1.addAtom(mol.getAtom(0));
pv1.addAtom(mol.getAtom(1));
pv1.addAtom(mol.getAtom(2));
pv1.addAtom(mol.getAtom(3));
pv1.addAtom(mol.getAtom(4));
pv1.addAtom(mol.getAtom(5));

Sgroup pv2 = new Sgroup();
pv2.setType(SgroupType.ExtMulticenter);
pv2.addAtom(mol.getAtom(8));
pv2.addBond(mol.getBond(7));
pv2.addAtom(mol.getAtom(0));
pv2.addAtom(mol.getAtom(1));
pv2.addAtom(mol.getAtom(2));
pv2.addAtom(mol.getAtom(3));
pv2.addAtom(mol.getAtom(4));
pv2.addAtom(mol.getAtom(5));

mol.setProperty(CDKConstants.CTAB_SGROUPS, Arrays.asList(pv1, pv2));
new DepictionGenerator().depict(mol).writeTo("~/tmp.svg");

Still more to do

  • V3000 reading improvements include Sgroups (writer supports them)
  • Multiple Group layout, this one might be tricky but should be possible

@johnmay
Correct ring peeling for Spiro rings.
db5baba
@johnmay
Additional custom template.
3630dfd
@johnmay
Placing Sgroup brackets.
c7432c0
@johnmay
Sgroup extension for multicenter bonds, Ctab v3000 can encode but not…
0ca4975 
… as an Sgroup.
@johnmay
Hide 'virtual' atoms in multicenter sgroups.
ae9cd7e
@johnmay
Layout multi center Sgroups.
d72007e
@johnmay
Attachment point representation as a value on pseudo atoms.
21cb634
@johnmay
Attachment point rendering.
994f192
@johnmay
Improved bracket layout - also spotted a typo in round bracket genera…
5028e39 
…tion.
@johnmay
Increase positional variation bond offset.
49b42ec
@johnmay
Tweaks to positional variation, now see two types of hiding is useful…
e732f81 
…, the 'fully' method means for atoms we skip the symbol generation.
@johnmay
Align attach point wave to the tip of bonds by drawing it in two sect…
213c891 
…ions.
@johnmay
Exclude tokens that are only prefixes.
378d50b
@johnmay
Round trip positional variation in Ctab V3000.
e336783
@johnmay
Copy link
Member Author

johnmay commented Nov 10, 2015

Relevant bit from the recommendations -
image

@johnmay
Copy link
Member Author

johnmay commented Nov 14, 2015

Added one more commit for handling the multiple group sgroups. Tricky to lay out as need to match substructures.

@johnmay
Resolve label alignment of multiple groups, we still hide bonds but t…
c1ed381 
…his should change text alignment as it would for abbreviations.
@johnmay
Copy link
Member Author

johnmay commented Nov 21, 2015

Spotted something whilst writing latest blog post - so tagged on one more commit.

egonw
egonw reviewed Nov 22, 2015
View reviewed changes
storage/io/src/test/java/org/openscience/cdk/io/MDLV3000ReaderTest.java
@@ -117,4 +125,13 @@ public void testPseudoAtomLabels() throws Exception {
}
}

@Test public void positionalVariation() throws Exception {
MDLV3000Reader reader = new MDLV3000Reader(getClass().getResourceAsStream("multicenterBond.mol"));
IAtomContainer container = reader.read(new org.openscience.cdk.AtomContainer(0,0,0,0));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I always get warnings that the reader is not closed... do you have any idea if doing or not has an effect on how fast the tests run? (just wondering...)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I should have closed it there. The main problem is you can only have a certain number of file pointers open on the OS.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI - just added a hotfix before release that resolves it.

egonw added a commit that referenced this pull request Nov 22, 2015
@egonw
Merge pull request #173 from cdk/patch/sgrouplayout
39e87ca 
Looks good to me.
@egonw egonw merged commit 39e87ca into master Nov 22, 2015
@johnmay johnmay deleted the patch/sgrouplayout branch June 5, 2016 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@johnmay @egonw