hiclib / pastis

Poisson-based algorithm for stable inference of DNA Structure
http://hiclib.github.io/pastis/
Other
34 stars 15 forks source link

Toy Example #77

Closed mozesjacobs closed 3 years ago

mozesjacobs commented 3 years ago

Pull request for the toy example

codecov[bot] commented 3 years ago

Codecov Report

Merging #77 (31e2e05) into master (d24d8de) will not change coverage. The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #77   +/-   ##
=======================================
  Coverage   47.47%   47.47%           
=======================================
  Files          52       52           
  Lines        4223     4223           
=======================================
  Hits         2005     2005           
  Misses       2218     2218           
Impacted Files Coverage Δ
pastis/io/read/all_data.py 74.00% <0.00%> (ø)
pastis/optimization/mds.py 40.90% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d24d8de...31e2e05. Read the comment docs.

NelleV commented 3 years ago

Hi @mozesjacobs I'm going to review & merge the other pull request first, as the commits of the first PR are mixed in with this one. In general, when you start working on a new bug, feature,or improvement it's best to start the history from where master is unless one depends on the other. I think the issue here is that you worked on the cooler pull request from master and thus when you created the branch "mozes," you thus included the commits of the master branch. If this occurs, you can create the branch from a remote branch: if you assume the remote from your fork of pastis is named origin on your computer and that you have a remote called pastis refering to hiclib/pastis's github repository, then you can create a branch from hiclib/pastis's master using git checkout -b pastis/master.

I'll go ahead and add a couple of notes in the example but overall it looks good!

mozesjacobs commented 3 years ago

Thanks Nelle! I will implement the changes u suggested. That should the only file, yep!

mozesjacobs commented 3 years ago

@NelleV The only flake8 thing left is: "./plot_inferred_structure.py:137:5: E731 do not assign a lambda expression, use a def". I am not entirely sure how to convert it to a definition - it's used in the cmap function I copy pasted. I can look into how to change it if need be.

I added this line to my import of Axes3d: "# noqa: F401 unused import" to suppress that flake8 warning about an unused import (I needed Axes3d to plot in 3d but don't actually use it explicitly).

mozesjacobs commented 3 years ago

@NelleV okay, I fixed the flake8!

NelleV commented 3 years ago

Thanks! I've cleaned up the history and merged the PR in #78