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

Contact trajectories #81

Merged
merged 21 commits into from
Sep 1, 2020
Merged

Contact trajectories #81

merged 21 commits into from
Sep 1, 2020

Conversation

dwhswenson
Copy link
Owner

@dwhswenson dwhswenson commented Aug 13, 2020

The main purpose of this is to introduce a ContactTrajectory subclass of ContactObject. It also comes with a few related additions and tools. Major step toward #53.

ContactTrajectory

This is essentially a list of single-frame ContactFrequency instances. A contact map for a specific frame can be obtained by either:

# these are the same
contacts = contact_trajectory[frame_num].residue_contacts
contacts = contact_trajectory.residue_contacts[frame_num]

The normal ContactTrajectory is a Sequence; there is also a MutableContactTrajectory which is a MutableSequence.

This PR will not include the movie making aspects suggested in #53 -- that will be left for future work.

ContactObject.from_contacts

For efficiency, the easiest way to calculate the contact trajectory is to have a loop that calculates the contacts for each frame, and then to create a ContactFrequency from atom_contacts and residue_contacts. We did not have a method to create a ContactFrequency from contacts before, so I added it. Like many things, it isn't allowed for ContactDifference, where it is overridden with a NotImplementedError.

The input to ContactObject.from_contacts can be either ContactCount instances, or the underlying collections.Counters (as well as, of course, the necessary information for ContactObject.__init__).

WindowedIterator and ContactTrajectory-specific implementation

The basic idea here is to facilitate rolling-average type behavior (perhaps in our case, rolling-frequency). By iterating over an underlying sequence, the idea is to return sequence indices that should be added to/subtracted from the rolling average. There's a general implementation (perhaps useful in other code!) and a specific use of it to get rolling contact frequencies.


Tasks

  • Implement and test ContactObject.from_contacts
  • Implement core of ContactTrajectory
  • Test core of ContactTrajectory
  • Test with sliced trajectory
  • Add compatibility checks where relevant in ContactTrajectory, with tests
  • Implement MutableContactTrajectory, with tests
  • WindowedIterator and tests
  • RollingContactFrequency and tests
  • Docstrings
  • Add example
  • Add to docs

@dwhswenson
Copy link
Owner Author

Quick comments on the codeclimate complaints:

  • from_contacts has too many arguments (3 times): True, but unavoidable. And I'll also make the arguments that, (1) most of those arguments typically use defaults; (2) while this is part of the API, it's aimed less at normal users than at developers. Both of these make the too-many-arguments thing less of a problem in my mind.
  • file contact_maps.py is too long: Yes. I'm not going to change that in this PR, but I was already thinking about reorganizing some of the code here. That will be a separate PR.
  • TODO: yes, I added a TODO... that shouldn't be blocking, though

I'll silence those issues for now: wontfix on the too many arguments, confirmed on the too long file. Approving the changes.

@dwhswenson dwhswenson marked this pull request as ready for review August 31, 2020 12:43
@dwhswenson
Copy link
Owner Author

This is ready for review. Once this one is in, I'll cut 0.6.0.

@dwhswenson dwhswenson requested a review from sroet August 31, 2020 12:43
@dwhswenson dwhswenson changed the title [WIP] Contact trajectories Contact trajectories Aug 31, 2020
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 codewise, some doc changes and leftover debug code in the tests

contact_map/contact_trajectory.py Outdated Show resolved Hide resolved
contact_map/contact_trajectory.py Outdated Show resolved Hide resolved
contact_map/contact_trajectory.py Outdated Show resolved Hide resolved
contact_map/contact_trajectory.py Outdated Show resolved Hide resolved
contact_map/tests/test_contact_trajectory.py Outdated Show resolved Hide resolved
contact_map/tests/test_contact_trajectory.py Outdated Show resolved Hide resolved
@dwhswenson
Copy link
Owner Author

Made changes as suggested, @sroet. Thanks for the quick review!

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, feel free to merge on green

@dwhswenson dwhswenson merged commit 6063210 into master Sep 1, 2020
@dwhswenson dwhswenson deleted the contact_trajectory branch September 1, 2020 06:43
@dwhswenson dwhswenson mentioned this pull request Sep 1, 2020
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