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

Proposed improvements to 2D Drawing Code #2931

Closed
DavidACosgrove opened this issue Feb 4, 2020 · 12 comments
Closed

Proposed improvements to 2D Drawing Code #2931

DavidACosgrove opened this issue Feb 4, 2020 · 12 comments
Milestone

Comments

@DavidACosgrove
Copy link
Collaborator

MedChemica have asked me to make some improvements to the 2D drawing code:

  1. Clipping of some structures - in general smaller molecules are ‘expanded’ to fit the box such that edge atoms (Cl, OH examples) are clipped off.

  2. OH NH (heteroatoms-protonated) tail groups align bond to the heteroatom (currently centred between the two atoms e.g. OH)

  3. OH and NH (heteroatoms-protonated) - the 'wrong way' around in ring systems of Left-to-right orientation (up/down, R to L). An example is N1CCC(CC1)c1ccccc1 where the N should be in the ring, the H above it, rather than the NH both being in the ring. You could make a case for a bigger font, as well, I think.
    Screenshot 2020-02-04 at 09 37 44

  4. Define fixed bond length and font size, and only scaling down when molecule exceeds the size of a defined box. No defined box then depiction is larger.

If time allows:
5. Allow highlighting of atoms by 2 colours, so that, for example, 2 SMARTS hits that overlap an atom can be shown.

4 and 5 will require changes to the API, so there is an obvious commitment not to break existing code.
MedChemica have a limited budget for this, but we could consider additional enhancements along these lines if they were quick to implement.

@greglandrum
Copy link
Member

These sound reasonable and sensible to me and would certainly be welcome!
I will comment on each of the suggested changes individually.

@greglandrum
Copy link
Member

  1. Clipping of some structures - in general smaller molecules are ‘expanded’ to fit the box such that edge atoms (Cl, OH examples) are clipped off.

I think a useful extension of this would be to allow a maximum bond length to be defined in order to prevent things molecules like CCC from filling a 300x300px window.

@greglandrum
Copy link
Member

  1. OH NH (heteroatoms-protonated) tail groups align bond to the heteroatom (currently centred between the two atoms e.g. OH)
  2. OH and NH (heteroatoms-protonated) - the 'wrong way' around in ring systems of Left-to-right orientation (up/down, R to L). An example is N1CCC(CC1)c1ccccc1 where the N should be in the ring, the H above it, rather than the NH both being in the ring. You could make a case for a bigger font, as well, I think.

Yes, I agree with both of these.

@greglandrum
Copy link
Member

  1. Define fixed bond length and font size, and only scaling down when molecule exceeds the size of a defined box. No defined box then depiction is larger.

I don't think this requires an API change. That sounds like additional parameters for MolDrawOptions.

The possible exception to that is caused by No defined box then depiction is larger., which I don't understand. Allowing the current implementation to draw onto a canvas of unspecified size would be non-trivial. (Certainly doable: we could render every structure into a unit square and then scale that appropriately to fit the windows, but I think that's probably a fair amount of work).

@greglandrum
Copy link
Member

  1. Allow highlighting of atoms by 2 colours, so that, for example, 2 SMARTS hits that overlap an atom can be shown

This would look really cool. If we can figure out an API for this it, I'd be happy to do the implementation work if you run out of time.

@DavidACosgrove
Copy link
Collaborator Author

DavidACosgrove commented Feb 4, 2020

"I don't think this requires an API change. That sounds like additional parameters for MolDrawOptions."
Good point. There's a lot to be said for the options classes.

"The possible exception to that is caused by No defined box then depiction is larger., which I don't understand."
Me neither, now I come to think about it. I will get clarification from Ed and Al.
Clarification - what they mean is indeed your extra suggestion - it will scale down from a given size if the molecule is too big for the box, but not up so the size becomes an upper limit.

" I think a useful extension of this would be to allow a maximum bond length to be defined in order to prevent things molecules like CCC from filling a 300x300px window."
This could come as a by-product of suggestion 4, probably.

@DavidACosgrove
Copy link
Collaborator Author

  1. Allow highlighting of atoms by 2 colours, so that, for example, 2 SMARTS hits that overlap an atom can be shown

This would look really cool. If we can figure out an API for this it, I'd be happy to do the implementation work if you run out of time.

I'd be happy to have the help, although I am currently thinking this might be the most fun bit. I haven't thought about the API yet. The aim is something like:
Screenshot 2020-02-04 at 13 44 48
We might aim for straighter lines. I am favouring the 2nd option at the moment.

@bp-kelley
Copy link
Contributor

bp-kelley commented Feb 4, 2020 via email

@edgriffen
Copy link

The tube style depiction is awesome.

On Tue, Feb 4, 2020 at 8:46 AM David Cosgrove @.***> wrote: 1. Allow highlighting of atoms by 2 colours, so that, for example, 2 SMARTS hits that overlap an atom can be shown This would look really cool. If we can figure out an API for this it, I'd be happy to do the implementation work if you run out of time. I'd be happy to have the help, although I am currently thinking this might be the most fun bit. I haven't thought about the API yet. The aim is something like: [image: Screenshot 2020-02-04 at 13 44 48] https://user-images.githubusercontent.com/9198870/73750150-958b9800-4754-11ea-8e10-1c0267129acd.png We might aim for straighter lines. I am favouring the 2nd option at the moment. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2931?email_source=notifications&email_token=ACLOFIGQK7GYLMQRJVNDTPLRBFWUDA5CNFSM4KPTSY62YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKXVPII#issuecomment-581916577>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACLOFIAV2FPQTBTOTNUVI5DRBFWUDANCNFSM4KPTSY6Q .

Thanks for the feedback on the 'tube map' style - I should have described it as 'subway map' style, but being a Londoner by birth, 'tube' sticks. Why create a new form of visualisation when there's a visual grammar already established? It's probably good for 2 sets of colouring, moderate for 3 , borderline for 4. But most of the use cases are probably going to be 2 so it may be fit for purpose. More suggestions for how to colour multi atom groups welcome, we're not wedded to that.

@DavidACosgrove
Copy link
Collaborator Author

Two colour looks neat in this representation because you can put a strip of colour down either side of a bond. Three or more colours would look less attractive, I think. Also, I expect that a first pass would have the same colour on either side of a bond that was only in one set, unlike as drawn in your demonstration, as long as that was acceptable. I can anticipate fiddliness making the colours contiguous down either side of the a line of bonds, as with your red.
Now I've thought about it a bit more, I think this represenatation would be easier to implement than the lasso one as you could treat each atom or bond independently of the rest without having to calculate arc intersections and horrors like that.

@greglandrum
Copy link
Member

greglandrum commented Feb 5, 2020

And for at least the terminal atoms you could align the "split circles" with the incoming bonds
I deleted the comment where I tried to illustrate this... it's better if I don't try and do such things. :-)

@greglandrum
Copy link
Member

Closed in #2979

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants