Add export to RDKIT to allow for chemdraw style visualization. #1137

chrisiacovella · 2023-07-31T00:21:02Z

PR Summary:

This adds in a function to export to an RDKIT RWMol (See issue #1138). This will allow a user to display a chemdraw style version of an atomistic compound. To enable structures to be drawn with correct bond order, we can now store the bond order in the bond graph as a data attribute. If you initialize a compound with RDKIt (i.e., from smiles) it will set the bond order into the bond list. the add_bond routine now accepts bond order as an argument, so one could manually define a molecule with correct bond order as well. If bond order is not defined, the data will be set to 'default' (which to_rdkit will assume means bond order 1). I didn't want to set this to 1 so as to make it clear to a user that this value was not actually set during construction.

I've added in tests to check the conversion and ensure that bond order is properly propagated when, e.g., we clone a compound. The default behavior is not changed (e.g., calling bonds() will just return the pair unless return_bond_order is set to true), so this should not break any existing functionality.

PR Checklist

Includes appropriate unit test(s)
Appropriate docstring(s) are added/updated
Code is (approximately) PEP8 compliant
Issue(s) raised/addressed?

for more information, see https://pre-commit.ci

mbuild/compound.py

…into rdkit_vis

codecov · 2023-07-31T19:56:42Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7142f41) 87.13% compared to head (0dd8fa6) 87.19%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1137      +/-   ##
==========================================
+ Coverage   87.13%   87.19%   +0.06%     
==========================================
  Files          62       62              
  Lines        6457     6490      +33     
==========================================
+ Hits         5626     5659      +33     
  Misses        831      831

Files	Coverage Δ
mbuild/compound.py	`97.13% <100.00%> (+0.01%)`	⬆️
mbuild/conversion.py	`89.80% <100.00%> (+0.39%)`	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

chrisiacovella · 2023-07-31T20:35:33Z

I was just thinking that since force overlap can create a bond, I need to update that function to allow bond order to be set when calling force overlap to create a bond. Again, mBuild won't necessarily do anything with bond order at this point, but I think adding the ability to set this consistently.

Since this isn't very high priority, I think adding in a simple "infer_bond_order" could be useful; I tried to use the function in rdkit that supposedly does this, but ran into an error, so I still need to look more closely into that.

I haven't yet tried to convert a non-atomistic particle to rdkit. I feel as this is doable, since we will explicitly set the bonds (i.e., these aren't a function of atomic species or something) and in the function I'm overwriting the names anyway to include an index. Basically just need to add some logic to know what to do if element isn't set and the name can't be parsed into an element.

daico007

Looks great! Only some minor comments about type check and where the conversion method itself should stay.

daico007 · 2023-08-01T00:25:15Z

mbuild/compound.py

+        if bond_order is None:
+            bond_order = "default"
+        self.root.bond_graph.add_edge(
+            particle_pair[0], particle_pair[1], bo=bond_order
+        )


Do you think we need a type check here to make sure the bond_order provided is of the right type (not bogus string)?

Yeah, I think hard checking these would be useful. The options should be:
"default", "single", "double", "triple", "aromatic", "unspecified". It seems the current options are:
"default", 1.0, 2.0, 3.0, 1.5, 0.0.
I think being cleaner with using all strings would be nice, since we wouldn't have to deal with odd behavior for floats vs ints.

Also, these options should be all listed in the doc strings to_rdkit(), regardless of what they are.

I added a check for bond_order using the types you suggested.

mbuild/compound.py

daico007 · 2023-08-01T00:32:03Z

mbuild/compound.py

+            3.0: Chem.BondType.TRIPLE,
+            1.5: Chem.BondType.AROMATIC,
+            0.0: Chem.BondType.UNSPECIFIED,
+            "default": Chem.BondType.SINGLE,


I think we should raise a single warning here that the "default" bond with be converted to single bond in rdkit.

So, it's not clear to me what the use of having "default" is with the changes discussed of how to specify bond order types. Do we need to have "default" as an option here, or in add_bond if the default is always a single bond?

daico007 · 2023-08-01T00:37:16Z

I think we can just have the rdkit for atomistic system for now (initial check for particle element as you have now should be sufficient). The conversion to CG systems could follow later. I think the force_overlap will call the add_bond method itself, so for now it will be set as "default", but having the option to set the bond order when calling force_overlap would be nice (and not too hard to implement I believe).

…tring formatting

CalCraven · 2023-10-19T16:12:59Z

mbuild/conversion.py

@@ -1749,6 +1757,59 @@
    return pybelmol


+def to_rdkit(compound):
+    """Create an RDKit RWMol from an mBuild Compound."""
+    rdkit = import_("rdkit")


Doc strings can probably get added here as well. Should add that we're returning Chem.RWmol, and maybe some examples of useful things to do with the mol and references to rdkit.

Possible examples.

from rdkit.Chem import Draw img = Draw.MolToImage(temp_mol) # draw a 2D structure Chem.MolToSmiles(temp_mol) # get smiles string Chem.RemoveHs(temp_mol) # remove Hydrogens # list of possible rdkit writers # link to other working with molecules info (https://www.rdkit.org/docs/GettingStartedInPython.html#working-with-molecules)

Good idea. I only added the draw example and the link since mBuild has functionality to remove particles and convert a compound to smiles.

chrisjonesBSU · 2023-10-19T00:16:29Z

@chrisiacovella I'm taking over this PR :)

I've addressed some of the comments from @daico007 and @CalCraven. I haven't looked closely yet at the unit tests, but I can work on adding a few more (and making sure the changes made aren't breaking the ones you added)

mbuild/tests/test_compound.py

for more information, see https://pre-commit.ci

chrisiacovella · 2023-10-19T17:46:32Z

Thanks @chrisjonesBSU I'll take a look at this. Not going to lie...I thought we merged this a while ago!

chrisjonesBSU · 2023-10-19T19:00:20Z

Ok, I think this should be ready to go. I added a unit test for the missing lines in to_rdkit that checked for particle elements and threw an error if none are found. It seems like converting to rdkit should only be done for atomistic systems, so I added a note about that in to_rdkit in Compound

chrisiacovella · 2023-10-20T17:25:49Z

Thanks for finishing this off. Everything looks good. I think we are ready to merge.

CalCraven

Thanks Chris, this looks done to me as well.

Christopher Iacovella and others added 9 commits June 8, 2023 21:49

fixed freud bond issue

cf759f1

[pre-commit.ci] auto fixes from pre-commit.com hooks

457b6ad

for more information, see https://pre-commit.ci

Merge branch 'main' into findbonds

2f35d1a

Update mbuild/compound.py

0096908

added to_rdkit function with bond order

5302b09

Merge branch 'mosdef-hub:main' into rdkit_vis

ec1a58f

updated bond functions to optionally accept/return bond order

332205b

added tests for bond order and converting to rdkit

fd1ed75

[pre-commit.ci] auto fixes from pre-commit.com hooks

127be7d

for more information, see https://pre-commit.ci

chrisiacovella changed the title ~~Rdkit vis~~ Add export to RDKIT to allow for chemdraw style visualization. Jul 31, 2023

github-advanced-security bot found potential problems Jul 31, 2023

View reviewed changes

mbuild/compound.py Fixed Show fixed Hide fixed

chrisiacovella added 3 commits July 31, 2023 12:33

fixed typo in test

fa7046d

Merge branch 'rdkit_vis' of https://github.com/chrisiacovella/mbuild …

7d75821

…into rdkit_vis

fixed typo in test that was missed in last fix

acba7c3

chrisiacovella added WIP Work in Progress, not ready for merging enhancement labels Jul 31, 2023

daico007 reviewed Aug 1, 2023

View reviewed changes

chrisjonesBSU added 4 commits October 18, 2023 16:56

Merge branch 'main' into rdkit_vis

0e8cbe7

Check value of bond_order and throw error if invalid, fix typos and s…

3adb518

…tring formatting

move rdkit conv code to conversion

457ae50

replace self with compound

a4b3921

github-advanced-security bot found potential problems Oct 19, 2023

View reviewed changes

update keys of bond order dict

fc023c1

CalCraven reviewed Oct 19, 2023

View reviewed changes

mbuild/tests/test_compound.py Outdated Show resolved Hide resolved

chrisjonesBSU and others added 2 commits October 19, 2023 10:34

fix dictionary names and keys

510f27d

[pre-commit.ci] auto fixes from pre-commit.com hooks

2ef402c

for more information, see https://pre-commit.ci

add new unit tests, check for type in add_bond

c580b00

chrisjonesBSU added 3 commits October 19, 2023 12:40

expand doc strings

fa6135f

add link to rdkit getting started page

4b14683

add unit test for error in rdkit

0dd8fa6

chrisjonesBSU removed the WIP Work in Progress, not ready for merging label Oct 19, 2023

chrisjonesBSU requested review from daico007 and CalCraven October 19, 2023 19:22

chrisjonesBSU linked an issue Oct 19, 2023 that may be closed by this pull request

Add to_rdkit functionality #1138

Closed

CalCraven approved these changes Oct 20, 2023

View reviewed changes

daico007 approved these changes Oct 20, 2023

View reviewed changes

daico007 merged commit c426c3a into mosdef-hub:main Oct 20, 2023
15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add export to RDKIT to allow for chemdraw style visualization. #1137

Add export to RDKIT to allow for chemdraw style visualization. #1137

chrisiacovella commented Jul 31, 2023 •

edited

Loading

codecov bot commented Jul 31, 2023 •

edited

Loading

chrisiacovella commented Jul 31, 2023

daico007 left a comment

daico007 Aug 1, 2023

CalCraven Oct 9, 2023

chrisjonesBSU Oct 18, 2023

daico007 Aug 1, 2023

chrisjonesBSU Oct 19, 2023

daico007 commented Aug 1, 2023

CalCraven Oct 19, 2023

chrisjonesBSU Oct 19, 2023 •

edited

Loading

chrisjonesBSU commented Oct 19, 2023

chrisiacovella commented Oct 19, 2023

chrisjonesBSU commented Oct 19, 2023

chrisiacovella commented Oct 20, 2023

CalCraven left a comment

Add export to RDKIT to allow for chemdraw style visualization. #1137

Add export to RDKIT to allow for chemdraw style visualization. #1137

Conversation

chrisiacovella commented Jul 31, 2023 • edited Loading

PR Summary:

PR Checklist

codecov bot commented Jul 31, 2023 • edited Loading

Codecov Report

chrisiacovella commented Jul 31, 2023

daico007 left a comment

Choose a reason for hiding this comment

daico007 Aug 1, 2023

Choose a reason for hiding this comment

CalCraven Oct 9, 2023

Choose a reason for hiding this comment

chrisjonesBSU Oct 18, 2023

Choose a reason for hiding this comment

daico007 Aug 1, 2023

Choose a reason for hiding this comment

chrisjonesBSU Oct 19, 2023

Choose a reason for hiding this comment

daico007 commented Aug 1, 2023

CalCraven Oct 19, 2023

Choose a reason for hiding this comment

chrisjonesBSU Oct 19, 2023 • edited Loading

Choose a reason for hiding this comment

chrisjonesBSU commented Oct 19, 2023

chrisiacovella commented Oct 19, 2023

chrisjonesBSU commented Oct 19, 2023

chrisiacovella commented Oct 20, 2023

CalCraven left a comment

Choose a reason for hiding this comment

chrisiacovella commented Jul 31, 2023 •

edited

Loading

codecov bot commented Jul 31, 2023 •

edited

Loading

chrisjonesBSU Oct 19, 2023 •

edited

Loading