-
Notifications
You must be signed in to change notification settings - Fork 271
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
[compute_contacts] new option for optarg scheme="backbone", tests added #1832
base: master
Are you sure you want to change the base?
Conversation
Hi 👋 @sukritsingh greetings from Berlin! |
mdtraj/geometry/contact.py
Outdated
if scheme not in ['ca', 'closest', 'closest-heavy', 'sidechain', 'sidechain-heavy']: | ||
raise ValueError('scheme must be one of [ca, closest, closest-heavy, sidechain, sidechain-heavy]') | ||
|
||
if scheme not in ['ca', 'closest', 'closest-heavy', 'sidechain', 'sidechain-heavy', "backbone"]: |
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.
Probably good to use single quotes here for backbone
to keep it consistent with the rest of the options (looks like double quotes used)
mdtraj/geometry/contact.py
Outdated
@@ -181,7 +184,7 @@ def compute_contacts(traj, contacts='all', scheme='closest-heavy', ignore_nonpro | |||
distances = md.compute_distances(traj, atom_pairs, periodic=periodic) | |||
|
|||
|
|||
elif scheme in ['closest', 'closest-heavy', 'sidechain', 'sidechain-heavy']: | |||
elif scheme in ['closest', 'closest-heavy', 'sidechain', 'sidechain-heavy', "backbone"]: |
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.
As above - change to single quotes
mdtraj/geometry/contact.py
Outdated
@@ -192,6 +195,9 @@ def compute_contacts(traj, contacts='all', scheme='closest-heavy', ignore_nonpro | |||
elif scheme == 'sidechain': | |||
residue_membership = [[atom.index for atom in residue.atoms if atom.is_sidechain] | |||
for residue in traj.topology.residues] | |||
elif scheme == "backbone": |
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.
change to single quotes
@@ -38,23 +38,27 @@ def test_contact_0(get_fn): | |||
closest_heavy, closest_heavy_pairs = md.compute_contacts(pdb, contacts, scheme='closest-heavy') | |||
sidechain, sidechain_pairs = md.compute_contacts(pdb, contacts, scheme='sidechain') | |||
sidechain_heavy, sidechain_heavy_pairs = md.compute_contacts(pdb, contacts, scheme='sidechain-heavy') | |||
backbone, backbone_pairs = md.compute_contacts(pdb,contacts,scheme="backbone") |
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.
change to single quotes
tests/test_contact.py
Outdated
|
||
ref_ca = np.loadtxt(get_fn('cc_ca.dat')) | ||
ref_closest = np.loadtxt(get_fn('cc_closest.dat')) | ||
ref_closest_heavy = np.loadtxt(get_fn('cc_closest-heavy.dat')) | ||
ref_sidechain = np.loadtxt(get_fn('cc_sidechain.dat')) | ||
ref_sidechain_heavy = np.loadtxt(get_fn('cc_sidechain-heavy.dat')) | ||
ref_backbone = np.loadtxt(get_fn("backbone.dat")) |
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.
maybe rename the file to cc_backbone.dat
for consistency with the other filenames?
This is a nice addition to the I added some minor comments/edits to keep things consistent with the rest of the code, but the rest of it looks good! |
Adds option
scheme="backbone"
for md.compute contactsAdds tests and one test file
backbone.dat
created like this: