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

Fix issue #292 #293

Merged
merged 8 commits into from
Feb 13, 2025
Merged

Fix issue #292 #293

merged 8 commits into from
Feb 13, 2025

Conversation

lohedges
Copy link
Contributor

This PR closes #292, i.e. fixes the issues caused by the previous RDKIt "fix". The issue here is that we must only disable implicit hydrogens when using the customised MDAnalysis inference routine. If used for a general conversion, the use of this option is inconsistent with what RDKit does, e.g. on SDF read, which leads to incorrect stereochemistry when applying the standard sanitization routines.

Additionally, I've switched to just calling the standard RDKit sanitization function since we were missing a few steps. Given all the issues, I want to be as self-consistent as possible. We can separate the stages in future if we find further problems, or provide options to use a sub-set of the routines if needed.

  • I confirm that I have merged the latest version of devel into this branch before issuing this pull request (e.g. by running git pull origin devel): [y]
  • I confirm that I have added a changelog entry to the changelog (we will add a link to this PR as part of the review): [y]
  • I confirm that I have permission to release this code under the GPL3 license: [y]

@lohedges lohedges merged commit 60e74f9 into devel Feb 13, 2025
4 of 5 checks passed
@lohedges lohedges deleted the fix_292 branch February 13, 2025 15:34
lohedges added a commit that referenced this pull request Feb 13, 2025
lohedges added a commit that referenced this pull request Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] More RDKit conversion woes
1 participant