Skip to content

Add explicit hydrogens when loading in SMILES #1172

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

Merged
merged 5 commits into from
May 13, 2021

Conversation

justinGilmer
Copy link
Contributor

Previously, if a user decided to use parmed.rdkit.load_smiles to
generate a parmed structure, no explicit hydrogens are added to the
structure, this can be an issue if trying to generate hydrocarbon
systems.

This PR leverages the built-in rdkit method rdkit.Chem.AddHs to generate explicit
hydrogens for an rdkit mol. An additional boolean flag: hydrogens=True has been added to this method. Since this could break others' workflows, the default value is False to comply with current functionality.

Previously, if a user decided to use `parmed.rdkit.load_smiles` to
generate a `parmed` structure, no explicit hydrogens are added to the
structure, this can be an issue if trying to generate hydrocarbon
systems.

This PR leverages the built-in `rdkit` ability to generate explicit
hydrogens for an rdkit mol using `rdkit.Chem.AddHs` by passing a boolean
flag `hydrogens=True` to use this method. Since this could break others'
workflows, the default value is `False` to comply with current
functionality.
Copy link
Contributor

@swails swails left a comment

Choose a reason for hiding this comment

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

Thank you! This is a nice addition!

I actually think the functionality you added should be made the default and people that use it will have to deal with the backwards incompatibility (there are several in the upcoming 4.0 release).

justinGilmer and others added 3 commits May 11, 2021 20:35
Co-authored-by: Jason Swails <jason@entos.ai>
Co-authored-by: Jason Swails <jason@entos.ai>
@justinGilmer
Copy link
Contributor Author

@swails , I went through and made the necessary changes discussed above and the units tests are passing locally for me. There was very little modification of the codebase to support these changes 🎉 .

Let me know if there is anything else needed on my end! It seems like for builds to run it needs approval from a maintainer: https://github.com/ParmEd/ParmEd/actions/runs/836655356

@swails
Copy link
Contributor

swails commented May 13, 2021

Thanks! I approved and will merge when the tests pass.

@swails swails merged commit 144033f into ParmEd:master May 13, 2021
@justinGilmer justinGilmer deleted the feat/rdkit-addhs branch May 13, 2021 20:16
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.

2 participants