-
Notifications
You must be signed in to change notification settings - Fork 91
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
Visualize fixes #1751
Open
Yoshanuikabundi
wants to merge
14
commits into
main
Choose a base branch
from
visualize-fixes
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+119
−91
Open
Visualize fixes #1751
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
4471cb9
Fix Molecule.visualize() overrides
Yoshanuikabundi d934f2f
Fix path to Ipython display module
Yoshanuikabundi 81e7170
Correct docs and types re: show_all_hydrogens w/ backend='nglview'
Yoshanuikabundi 840dfd3
Clean up Molecule.visualize docstring and code
Yoshanuikabundi 4a3197e
remove ensure_correct_connectivity and update docstring
Yoshanuikabundi 6f05c45
Display multiple bonds when available
Yoshanuikabundi c003634
Document that bonds may be deduced from positions in mol.visualize()
Yoshanuikabundi e80b347
Write PDB with RDKit when visualizing topologies
Yoshanuikabundi 0dc93d0
Fall back to OpenMM if RDKit unavailable and document
Yoshanuikabundi 32e8700
Update changelog
Yoshanuikabundi 50bdb9a
Merge remote-tracking branch 'upstream/main' into visualize-fixes
mattwthompson 4ff6bab
Ignore missing type annotations in rdkit
Yoshanuikabundi 584a039
Merge branch 'main' into visualize-fixes
Yoshanuikabundi a57a88d
Merge branch 'main' into visualize-fixes
j-wags File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,12 @@ | ||
import math | ||
import uuid | ||
from io import StringIO | ||
from typing import TextIO | ||
|
||
from nglview.base_adaptor import Structure, Trajectory | ||
|
||
from openff.toolkit import Molecule, Topology, unit | ||
from openff.toolkit.utils.toolkits import RDKIT_AVAILABLE | ||
|
||
MOLECULE_DEFAULT_REPS = [ | ||
dict(type="licorice", params=dict(radius=0.25, multipleBond=True)) | ||
|
@@ -22,7 +25,7 @@ class MoleculeNGLViewTrajectory(Structure, Trajectory): | |
Parameters | ||
---------- | ||
molecule | ||
The `Molecule` object to display. | ||
The ``Molecule`` object to display. | ||
ext | ||
The file extension to use to communicate with NGLView. The format must | ||
be supported for export by the Toolkit via the `Molecule.to_file() | ||
|
@@ -70,16 +73,14 @@ class TopologyNGLViewStructure(Structure): | |
""" | ||
OpenFF Topology adaptor. | ||
|
||
Communicates with NGLView via PDB, using RDKit to write redundant CONECT | ||
records indicating multiple bonds. If RDKit is unavailable, falls back | ||
to ``Topology.to_file``. | ||
|
||
Parameters | ||
---------- | ||
topology | ||
The `Topology` object to display. | ||
ext | ||
The file extension to use to communicate with NGLView. The format must | ||
be supported for export by the Toolkit via the `Topology.to_file() | ||
<openff.toolkit.topology.Topology.to_file>` method, and import by | ||
NGLView. File formats supported by NGLView can be found at | ||
https://nglviewer.org/ngl/api/manual/file-formats.html | ||
The ``Topology`` object to display. | ||
|
||
Example | ||
------- | ||
|
@@ -92,15 +93,54 @@ class TopologyNGLViewStructure(Structure): | |
def __init__( | ||
self, | ||
topology: Topology, | ||
ext: str = "PDB", | ||
): | ||
self.topology = topology | ||
self.ext = ext.lower() | ||
self.ext = "pdb" | ||
self.params: dict = dict() | ||
self.id = str(uuid.uuid4()) | ||
|
||
def get_structure_string(self): | ||
with StringIO() as f: | ||
self.topology.to_file(f, file_format=self.ext) | ||
structure_string = f.getvalue() | ||
if RDKIT_AVAILABLE: | ||
from rdkit.Chem.rdmolfiles import PDBWriter # type: ignore | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (blocking) Direct cheminformatics toolkit imports outside of toolkit wrappers have caused all sorts of havoc before. Even with the RDKIT_AVAILABLE guard I'm worried about this. Can this be refactored to put the RDKit import and associated code in RDKitToolkitWrapper? |
||
|
||
write_box_vectors(f, self.topology) | ||
|
||
writer: PDBWriter = PDBWriter(f) | ||
for mol in self.topology.molecules: | ||
writer.write(mol.to_rdkit()) | ||
|
||
writer.close() | ||
|
||
structure_string = f.getvalue() | ||
else: | ||
self.topology.to_file(f, file_format="pdb") | ||
structure_string = f.getvalue() | ||
|
||
return structure_string | ||
|
||
|
||
def write_box_vectors(file_obj: TextIO, topology: Topology): | ||
if topology.box_vectors is not None: | ||
a, b, c = topology.box_vectors.m_as(unit.nanometer) | ||
a_length = a.norm() | ||
b_length = b.norm() | ||
c_length = c.norm() | ||
|
||
alpha = math.acos(b.dot(c) / (b_length * c_length)) | ||
beta = math.acos(c.dot(a) / (c_length * a_length)) | ||
gamma = math.acos(a.dot(b) / (a_length * b_length)) | ||
|
||
RAD_TO_DEG = 180 / math.pi | ||
print( | ||
"CRYST1%9.3f%9.3f%9.3f%7.2f%7.2f%7.2f P 1 1 " | ||
% ( | ||
a_length * 10, | ||
b_length * 10, | ||
c_length * 10, | ||
alpha * RAD_TO_DEG, | ||
beta * RAD_TO_DEG, | ||
gamma * RAD_TO_DEG, | ||
), | ||
file=file_obj, | ||
) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(not blocking) Is this the correct interpretation, or am I misunderstanding the intent here?