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

[WIP] Add a warning to Topology.from_pdb if the stereo of the molecule in the PDB is different from the substructure template or unique mol #1583

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

j-wags
Copy link
Member

@j-wags j-wags commented Apr 17, 2023

@codecov
Copy link

codecov bot commented Apr 17, 2023

Codecov Report

Merging #1583 (e42b162) into main (b528795) will decrease coverage by 0.23%.
The diff coverage is 97.14%.

Additional details and impacted files

Comment on lines +454 to +461
# TODO: This is most likely wrong. This will copy over CW/CCW tags but there's no
# checking that the substituent order in the molecule and substructure are the same,
# so this is basically random assignment. Thankfully Molecule.from_polymer_pdb and
# Topology.from_pdb both overwrite this assignment based on 3D coordinates. The only
# way I can currently think of doing this right would be to arbitrarily assign CW,
# then try to run a strict substructure match on it, and if that fails then assign
# CCW. This would be monstrously inefficient and all it would let us do is compare
# the substructure stereo to 3D stereo, which is of dubious value anyway.
Copy link
Member Author

Choose a reason for hiding this comment

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

Big blocker here, this basically summarizes it. This will need a lot more work and I don't think it's worth holding up the release for.

@j-wags j-wags mentioned this pull request Apr 26, 2023
4 tasks
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

1 participant