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

Support iTOL-style "leaf sorting" #170

Closed
fedarko opened this issue Jun 13, 2020 · 1 comment · Fixed by #394
Closed

Support iTOL-style "leaf sorting" #170

fedarko opened this issue Jun 13, 2020 · 1 comment · Fixed by #394
Assignees
Labels
needs-js-layouts Needs #213 to be taken care of first nice to have

Comments

@fedarko
Copy link
Collaborator

fedarko commented Jun 13, 2020

This would sort clades in the rectangular and circular layouts by the number of descendant leaves (see the iTOL docs here). I've seen this strategy used in lots of tree figures in papers, and it looks pretty :), so I assume it would be nice if this was an option. (Would be easiest to implement on the command line, but be useful to have in the browser.)

Currently, Empress doesn't do this kind of sorting -- so the original order of clades in the tree's Newick file is followed.

update: I've implemented this in the leafsorting branch of my fork (https://github.com/fedarko/empress/tree/leafsorting). I imagine it would be easiest to revisit this once the current prototype branch is finalized and merged in.

@fedarko
Copy link
Collaborator Author

fedarko commented Jun 13, 2020

For reference, here are some screenshots showing how this looks. There are three options:

  • no leaf sorting (how things are currently drawn in Empress)
  • leaf sorting, with clades sorted in descending order by number of leaves (how iTOL's "normal mode" works by default)
  • leaf sorting, with clades sorted in ascending order by number of leaves (opposite of the above, included here because I messed the sorting up the first time and thought it looked cool)

These are all on the moving pictures tree (which I admit may not be the best test case for this due to its handful of super long branches). I'm also including iTOL's visualizations of the tree for comparison (although that tree is uncolored, so this is less of a competition re: "omg which looks better????" and more of a "basically everyone uses iTOL because it is super useful, so let's see how closely our layout choices resemble theirs to save everyone's sanity").

Rectangular layout

No leaf sorting
image

With leaf sorting (descending order)
image

With leaf sorting (ascending order)
image

iTOL (had to set the Vert scaling factor to 0.05 to get the entire tree to display on the screen; I believe this is because Empress' rectangular layout uses a relatively small distance between tips' y-coordinates, whereas iTOL's default is much larger)
image

Circular layout

No leaf sorting
image

With leaf sorting (descending order)
image

With leaf sorting (ascending order)
image

iTOL (to be consistent with our circular layout as is, I changed the rotation to 0 degrees from default 210; also changed arc to 360 degrees from default 350)
image

Unrooted layout

(Substituting in the leaf-sorted postorder traversal for the postorder traversal in the unrooted layout doesn't seem to make a difference -- all of the Empress plots look basically the same, so I'm not posting an extra set of screenshots here.)

@ElDeveloper ElDeveloper added this to the Beta Release milestone Jul 7, 2020
@ElDeveloper ElDeveloper removed this from the Beta Release milestone Jul 27, 2020
@fedarko fedarko added the needs-js-layouts Needs #213 to be taken care of first label Aug 11, 2020
@fedarko fedarko mentioned this issue Aug 22, 2020
fedarko added a commit to fedarko/empress that referenced this issue Sep 24, 2020
fedarko added a commit to fedarko/empress that referenced this issue Sep 25, 2020
ElDeveloper pushed a commit that referenced this issue Sep 28, 2020
* ENH: Move layout options to layout tab: close #379

think node circles is ok where it is for now

* ENH: initial draft of leaf sorting in js

i mean it doesnt work yet, it just kinda screws up the tree

but it doesnt crash soooo

* BUG: fix child sorting

so leaf sorting kinda works now! untested but still

* ENH: Enable JS leaf sorting (close #170)

* ENH: update leaf sorting desc dynamically

* MNT: temporarily? remove layout avg point cache

was messing with leaf sorting, since two differently-sorted versions
of the same layout can take up drastically different parts of the
screen

* ENH: disable leaf sorting sel for unrooted layout

prevents user from clicking on it and it recentering the tree but
doing nothing

* ENH: Just hide leaf sorting rather than disabling

* DOC/PERF/MNT: cache l.s. results; imprv docs/code

* STY: prettify

* MNT: rm old todo

* DOC/PERF: Document new bptree funcs; speed up desc

* DOC: clean up BPTree.getSortedChildren()

* DOC: clarify postorederLeafSortedNodes docs

* TST: Test postorder leafsorted (basic tests)

* BUG: ignore root during circ layout

wasn't ignoring it right. argh! that's why tests are good :3

* TST: unbreak (most of) the js tests

* STY/TST/DOC: tidy up centerLayoutAvgPoint &fix tst

* STY: prettify layout tests

* DOC/TST: doc & tst LayoutsUtil.getPostOrderNodes()

* TST: expand leaf sorting test

* TST/DOC: Add tests+beef up docs for get*Children

I actually understand what "index" means inside BPTree now! nice.

* STY: prettify

* TST: Add rect layout leaf sorting test

* TST: Test desc leaf sorting via layout also kinda

* STY: prettify

* TST: kinda test leaf sorting for circ layouts

not very thorough but better than nothing

* DOC/BUG: doc some new sp funcs; fix desc text

* DOC: update README barplot sec now that #170 done

* TST: make centerLayoutAvgPt tests use util func

Suggestion c/o @ElDeveloper.

* BUG: add back accidentally-rm'd node circles txt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-js-layouts Needs #213 to be taken care of first nice to have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants