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

Add some features for Organometallic molecules #2616

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Jesnm01
Copy link

@Jesnm01 Jesnm01 commented Aug 4, 2023

Hi, it's the first time I work on an open-source project, and on top of that this is quite big, so I will probably need some help. I don't quite know the dynamics of change management in this kind of projects or who I have to contact to review these changes I propose.

These modifications have been made for my end-of-degree project with the objective of:

  • Creating a new canonical nomenclature that works more appropriately with organometallic compounds (placing the central metal of the molecule at the beginning of the canonical SMILES).
    Mention that molecules which do not contain transition metals are not affected by these changes.
  • Improve the visualization of compounds containing cyclopentadienyl (Cp) complexes.

So that it does not fall on deaf ears or go unnoticed, I propose the changes here so that they can eventually be merged if they are of enough interest.

Some results for the depictions can be seen in this folder, and this folder for the canonical SMILES. In the latter, you can see the difference between the canonicals that openbabel generated up to now, versus the canonicals that are generated now for organometallic compounds.

For further details consult my personal repository for my end-of-degree project and in particular the pdf document for the project.

Bear in mind that the pdf document as well as most of the explanatory comments that I make in the code are in Spanish, so if not familiar with the language it may be difficult to understand it directly. Most relevant sections of the pdf are chapter 4.2 and chapter 5.

Looking at the sizes of the other pull requests, I understand that the changes I propose are quite large and many features need to be revised or even rejected entirely.

These are the classes to which I added new variables or methods:

  • Added new features or variables to these classes:
    • OBMol
    • OBAtom
    • OBMol2Cansmi
    • OBCanSmiNode
    • SVGPainter (consecuently to OBPainter, ASCIIPainter and CommandPainter)
  • Added these new classes:
    • CpComplex: handle and define Cp (cyclopentadienyl) structures
    • BranchBlock: detect branches during the canonicalization process, mainly for Cp depiction purposes
    • OpCpDraw: plugin operation for identification and Cp storing inside the molecule for its later depiction.
    • SubTreeSizes: helping struct for new organometallic canonicalization method.
    • subtreecomp and mycomp: custom comparator objects for the new organometallic canonicalization method.
  • Modified some methods such as CreateCansmiString or OBDepict::DrawMolecule (among others) to fit in the new code. Modifications have comments above starting with '//New:' explaining them

In case its relevant, I've been working in Visual Studio C++ 2022 for Windows.

Let me know any concerns regarding the code.
Right now I don't have a lot of time to dedicate to this project, so I may not be able to quickly address any changes requested.

@welcome
Copy link

welcome bot commented Aug 4, 2023

Thanks for opening this pull request! Please check out our contributing guidelines and please examine any build issues.

@baoilleach
Copy link
Member

Regarding integration into OB (or any other open source project) t's usual to discuss planned large changes before the work is started so as to receive feedback and address concerns or consider alternatives. In particular, modifications to core classes are considered carefully. Creating a new canonical nomenclature is a very interesting project though, and impressive as an end of degree project. However it probably merits quite a bit of discussion both within OB and the wider cheminf community we are part of before it is integrated. Having a reference implementation provides a basis for that discussion

@ghutchis
Copy link
Member

ghutchis commented Aug 6, 2023

I'll echo @baoilleach - these look very interesting, and I'd be happy to see improved handling of organometallic / haptic structures.

But let's talk about the approach and alternatives on the list. You mentioned end-of-degree project. Are you willing to work a bit more on this?

@Jesnm01
Copy link
Author

Jesnm01 commented Aug 6, 2023

Referring to both comments, being a final degree project there were some time limits I had to stick to in order to finish it and make the changes (in about 3 to 5 months, including learning chemistry and understanding the base code from scratch).
That's why we were not really considering to include too much external feedback, which could have made this process more complex.

The project had specific goals, and in that time frame I came up with these solutions. I am by no means an expert in chemistry and my knowledge is quite scarce.
So you will probably find many things that can be improved, done in a more efficient way or better approaches that I might not have thought of.

I'm not quite available right now and I am planning on doing other things in the near future. But I could dedicate some of my free time to it, and slowly modify the code until its acceptable. So I am at your disposal, I will need some guidance for this.

@ChristianEdwardPadilla
Copy link

Hi I might be interested in taking up this PR. I have experience with software dev and organometallic chemistry.

Where does discussion take place for contributing to Open Babel? I see a development mailing list, but I'm not sure it's active. Is there a Slack/Discord somewhere?

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

Successfully merging this pull request may close these issues.

None yet

4 participants