ENH: heatmap #114
ENH: heatmap #114
Conversation
|
@RNAer this is finally ready for review! |
| @@ -49,7 +45,6 @@ class Dendrogram(TreeNode): | |||
|
|
|||
There was a problem hiding this comment.
did see where aspect_distorts_lengths is used? what does it do? is it neccessary?
There was a problem hiding this comment.
This was a direct port from pycogent. I'm very ok with dropping this. Thanks for catching!
gneiss/plot/_dendrogram.py
Outdated
| @@ -49,7 +45,6 @@ class Dendrogram(TreeNode): | |||
|
|
|||
| def __init__(self, use_lengths=True, **kwargs): | |||
| """ Constructs a Dendrogram object for visualization. | |||
|
|
|||
| """ | |||
| super().__init__(**kwargs) | |||
| self.use_lengths_default = use_lengths | |||
There was a problem hiding this comment.
if use_lengths_default is public, document it? I don't get what this is
There was a problem hiding this comment.
Also looks like this is a direct port. Dropping!
| @@ -105,7 +100,6 @@ def coords(self, height, width): | |||
| The height of the canvas. | |||
| width : int | |||
| The width of the canvas. | |||
|
|
|||
There was a problem hiding this comment.
I think you can delete line 79: self.length = self.length
|
|
||
| def _sign(x): | ||
| """Returns True if x is positive, False otherwise.""" | ||
| return x and x/abs(x) |
There was a problem hiding this comment.
not in the diff, but in line 38, the sentence is grammatically wrong and a bit confusing. length is the length from what to the subtree?
There was a problem hiding this comment.
I tried to improve the wording. Let me know what you think!
Removing attributes that aren't used
|
Thanks for the feedback @RNAer ! I tried to address your concerns. Let me know what you think. |
|
Changes Unknown when pulling b0caea4 on mortonjt:heatmap into ** on biocore:master**. |
|
Thanks! Would you like to merge?
…On Mar 2, 2017 11:30 AM, "Zech Xu" ***@***.***> wrote:
***@***.**** approved this pull request.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#114 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AD_a3ev0-m0gl9-AnKyRzM2Fmr4C840jks5rhxi8gaJpZM4MNuwh>
.
|
|
Thanks!
…On Thu, Mar 2, 2017 at 12:19 PM, Qiyun Zhu ***@***.***> wrote:
Merged #114 <#114>.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#114 (comment)>, or mute the
thread
<https://github.com/notifications/unsubscribe-auth/AD_a3V6GJN1nlFXRY2u0XXG_Jj2aks_5ks5rhyRmgaJpZM4MNuwh>
.
|
Heatmaps through matplotlib + dendrograms + metadata highlighting
🎆
This was pair-programmed with @amnona