Skip to content

Easy access to haystack/query residue indices and range#43

Merged
sroet merged 5 commits intomasterfrom
residues_for_atoms
Nov 13, 2018
Merged

Easy access to haystack/query residue indices and range#43
sroet merged 5 commits intomasterfrom
residues_for_atoms

Conversation

@dwhswenson
Copy link
Owner

The example notebook previously showed how to find the residues for query/haystack atoms, and then how to get the max/min needed for plotting that subset. But that was more complicated than it needed to be, especially because I'm finding that those "zoomed in" contact maps are something that I want frequently.

This PR adds:

  • query_residues / haystack_residues : list of the residue indices for the query/haystack
  • query_residue_range / haystack_residue_range : max and min index values of the above

This makes it easy to adjust the plot axes with, e.g.,

ax.set_xlim(*contacts.haystack_residue_range)

Includes tests; updates example notebook.

Copy link
Collaborator

@sroet sroet left a comment

Choose a reason for hiding this comment

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

LGTM, the only thing that had me thinking for a while:
the residue selection in done with

"resSeq 32 to 38"

Which is 1 based while the internal (&plot) index is 0 based

You might want to change to resid for the selection instead, for better matching.
selection ref: http://mdtraj.org/1.6.2/atom_selection.html

Copy link
Collaborator

@sroet sroet left a comment

Choose a reason for hiding this comment

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

LGTM, one final nitpick

@sroet
Copy link
Collaborator

sroet commented Nov 13, 2018

LGTM

@dwhswenson
Copy link
Owner Author

Made some implementation changes: I decided that these are better as properties, and that haystack_residues/query_residues should return the residues themselves (the mdtraj.core.topology objects) instead of just the indices.

@sroet: I think you have merge access now -- feel free to merge this after appveyor passes!

@sroet sroet merged commit 0aa03dd into master Nov 13, 2018
@dwhswenson dwhswenson deleted the residues_for_atoms branch November 13, 2018 14:53
@dwhswenson dwhswenson mentioned this pull request Jun 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants