Skip to content
This repository has been archived by the owner on Mar 20, 2020. It is now read-only.

I19 haploblocker #22

Merged
merged 38 commits into from
Aug 15, 2019
Merged

I19 haploblocker #22

merged 38 commits into from
Aug 15, 2019

Conversation

josiahseaman
Copy link
Member

This pull request rearranges the file structure to bring everything together under a unified django app structure. Torsten and Josiah's work on HaploBlocker is contained in the HaploBlocker app, while Toshiyuki and Josiah's work on Graph structures is contained in the Graph app. HaploBlocker is successfully using all 3 of the basic graph simplifications.

Currently, HaploBlocker and Graph are not using the same data objects. The next step will be a HaploBlocker refactor to use database objects defined in Graph.

Travis is currently not passing because the Django path changes were never resolved. However, unit tests run from PyCharm all pass (or are skipped).

  • Graph/test.py
  • HaploBlocker/tests.py

josiahseaman and others added 30 commits July 23, 2019 16:28
…simple creation test. Run manage.py migrate
Travis still won't be happy with the Django setup.
…blocker

# Conflicts:
#	HaploBlocker/HaploBlocker.ipynb
…blocker

# Conflicts:
#	HaploBlocker/HaploBlocker.ipynb
…lit_one_group: Added new test with by hand data. Refactoring of update_transitions.
…sts. Next is integration with the rest of vgbrowser
@josiahseaman josiahseaman added the enhancement New feature or request label Aug 13, 2019
Copy link
Member

@subwaystation subwaystation left a comment

Choose a reason for hiding this comment

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

Nice job @josiahseaman and @tpook92, thumps up!

There are some minor questions from my side regarding understanding the code and documentation of the code. I would be happy, if you could answer them ;)

As soon as I have a clear picture, you are ready to go.

Graph/admin.py Show resolved Hide resolved
Graph/models.py Show resolved Hide resolved
Graph/models.py Show resolved Hide resolved
Graph/models.py Show resolved Hide resolved
@@ -336,12 +341,3 @@ def load_from_slices(cls, slices, paths):
graph = cls(paths)
graph.slices = slices
return graph


if __name__ == "__main__":
Copy link
Member

Choose a reason for hiding this comment

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

How do we feed the location of the xg binary now? Not needed, yet?

HaploBlocker/tests.py Show resolved Hide resolved
HaploBlocker/haplonetwork.py Show resolved Hide resolved
HaploBlocker/haplonetwork.py Show resolved Hide resolved
class Node:
def __init__(self, ident, start, end, specimens=None, upstream=None, downstream=None):
self.ident = ident
self.start = start #Point()
Copy link
Member

Choose a reason for hiding this comment

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

So start and end are SNPs? Or SNPs and base pairs? Or a node id?
Here, I would find it really helpful, if you could add some multi line docstrings.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes these are SNP-numbers.
This originated from us using a SNP-chip data set to build the original graph. The only time they are actually used is for generating nodeIDs and to make it easier for us to check the results.

Do we need some kind of position for the visualization? BP-position might be different based on specimen or even multiple values for a specimens in case of CNV.

HaploBlocker/tests.py Show resolved Hide resolved
@ekg
Copy link

ekg commented Aug 14, 2019 via email

Copy link
Member

@subwaystation subwaystation left a comment

Choose a reason for hiding this comment

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

With the documentation added I can understand the code much better now.
I have no further questions from my side.

@@ -0,0 +1,343 @@
"""
Copy link
Member

Choose a reason for hiding this comment

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

Very detailed documentation now, thanks!

Graph/models.py Show resolved Hide resolved
README.md Show resolved Hide resolved
Graph/models.py Show resolved Hide resolved
HaploBlocker/tests.py Show resolved Hide resolved
@subwaystation subwaystation merged commit ea7750a into master Aug 15, 2019
@josiahseaman josiahseaman deleted the i19_haploblocker branch August 16, 2019 09:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants