Add an initial bridge from ccdata to rdkit Mol objects #1205
Conversation
Signed-off-by: Geoff Hutchison <geoff.hutchison@gmail.com>
|
Work in progress |
|
Is it worth grabbing all the atomic coordinates or an option to select which set of coordinates to use for bond perception? |
We don't standardize on this yet, but in a few places we take the approach of having Are you asking about that generally, or if bond perception should happen on a set of coordinates different from those initially loaded into the molecule (I guess a different rdkit conformer)? |
|
Using a specific index is a nice idea. I'll also add a parameter if someone wants to grab all the coordinates as separate conformers for some reason. I'm basically looking for feedback / code review. For example, should I check the RDKit version string rather than trying to import the bond perception module? |
|
Ok, I understand better now. Historically we did try/except for imports before adding this It's really up to you, but my thinking is that |
|
Bumping this to next release (1.8.1 tentatively), since we want to push out 1.8 soonish. |
|
Would it help to move this long if I wrote some tests? |
|
This got lost in a Slack chat, but I am wary of us merging any code that does not come with tests. |
|
If you can help get some tests started, I can make sure they provide good coverage. Sorry for this getting lost in the shuffle. |
|
I need to install rdkit in our CI images. |
No description provided.