search

biocore / empress

A fast and scalable phylogenetic tree viewer for microbiome data analysis
BSD 3-Clause "New" or "Revised" License
48 stars 31 forks source link

Tree controller #492

Closed kwcantrell closed 3 years ago

kwcantrell commented 3 years ago

This PR implements a new object TreeController that will allow empress to support dynamic shearing based on feature metadata or even sample metadata. TreeController will shear the tree based on a set of tip names (based on https://github.com/wasade/improved-octo-waddle/blob/0e9e75b77238acda6752f59d940620f89607ba6b/bp/_bp.pyx#L732). Also, to make integration of TreeController as seamless as possible, all input and output of TreeController will be w.r.t the original tree. In other words empress.js will not know if a tree has been sheared. Instead, TreeController will internally convert its input from the original tree to the sheared tree and back. This makes handling the metadata A LOT easier since empress indexes metadata using a nodes post-order position (w.r.t the non-sheared tree).

In addition, TreeController introduces iterators for the post-order and in-order traversals (now we no longer need to do for (i=1; i < this.tree.size; i++) to perform post-order traversal. Instead, we simply do for(node of this.tree.postorderTraversal()))

I've gone through a lot of different iterations of this PR so I am not sure how simple this final product turned out. So we can also setup a meeting if it makes reviewing this PR easier. :upside_down_face:

kwcantrell commented 3 years ago

Thanks @ElDeveloper for posting a live demonstration! Like you said this PR is not meant to handle recoloring, the legend, etc. That will all be handled in a separate PR that deals with the UI.

gwarmstrong commented 3 years ago

This looks awesome! Am I correct that it is not hooked up to the interface in any way yet?

ElDeveloper commented 3 years ago

Yep, you are correct! Thanks for looking over.

On (Mar-17-21|18:52), George Armstrong wrote:

This looks awesome! Am I correct that it is not hooked up to the interface in any way yet?

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://urldefense.com/v3/__https://github.com/biocore/empress/pull/492*issuecomment-801555810__;Iw!!Mih3wA!UkuNGX4xbqrOvyQKpeO9dDvnOOIhbdBgf6lurjSopkj5sQwbbOzCIcfLZAwf8uQ$

emperor-helper commented 3 years ago

The following artifacts were built for this PR: empire-biplot.qzv, empire.qzv, empress-tree.qzv, just-fm.qzv, plain.qzv

ElDeveloper commented 3 years ago

Thanks so much @kwcantrell !