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

Add support for circular layout barplots #297

Closed
fedarko opened this issue Aug 5, 2020 · 2 comments · Fixed by #357
Closed

Add support for circular layout barplots #297

fedarko opened this issue Aug 5, 2020 · 2 comments · Fixed by #357

Comments

@fedarko
Copy link
Collaborator

fedarko commented Aug 5, 2020

Continuation of #201, since the #293 PR only supports rectangular layouts.

@fedarko fedarko self-assigned this Aug 5, 2020
@ElDeveloper ElDeveloper added this to the Second Beta Release milestone Aug 5, 2020
@fedarko fedarko added the needs-js-layouts Needs #213 to be taken care of first label Aug 14, 2020
@fedarko
Copy link
Collaborator Author

fedarko commented Aug 15, 2020

planned drawing algorithm for a single layer (for now only focusing on feature metadata layers, but generalizing this to stacked sample metadata barplots shouldn't be too crazy) --

  • First off, figure out the colors and lengths assigned to each tip in this barplot layer. This isn't anything new.

  • Find the maximum-radius-from-the-root node in the tree (or if this isn't the first layer, the maximum-radius bar in the previous layer). This radius, plus some constant "gap" space, will be the minimum radius for this layer: i.e. all of this barplot layer's bars will begin at this radius. (This is analogous to the prevLayerMaxX used for drawing barplots in the rect. layout, which is the leftmost point for all bars in a layer).

  • There is a metric we can get by computing 2pi / # tips in the tree which I'm going to call cscf. This is analogous to the yrscf attribute used in the rectangular layout. Basically, this is the "maximum" angle we can give each individual tip's barplot wedge-thing without things starting to overlap. For a tree with two leaves this will be 2pi/2 = pi, for a tree with three leaves it'll be 2pi/3, etc. This only needs to be computed once for a given visualization (assuming the number of tips stays constant, i.e. if we add dynamic tree shearing or something in the future we'll need to recompute this).

  • For each tip, we need to draw a wedge-ish thing starting at an angle of tip.angle - (cscf/2) and ending at an angle of tip.angle + (cscf/2) (with the inner radius for the wedge being set to maxRadius and the outer radius being set to maxRadius + tip.length_for_this_barplot_layer). The effect of this is that each tip's wedge should look "centered" on the tip.

    • We can approximate this by drawing a bunch of rectangles (aka pairs of triangles) throughout this space*, similarly to how collapsed clades in the circular layout are drawn, but it would be ideal to use shaders to draw this as a circle with the requested area filled in. (In either case, I think we'd need to convert between Polar / Cartesian coordinates to figure out where to draw things -- shouldn't be too much work.)

Here's a very rough diagram of what this would look like for a tiny tree with 5 leaves -- the max-radius node is circled:
IMG_20200814_185049

* For small trees with just a handful of leaves, we'd need to draw lots of rectangles (maybe 15ish, like for arcs / collapsed clade wedges) per tip to prevent things from looking too jagged. For trees with more than a handful of leaves, maybe just 1-2ish rectangles will be fine.

@fedarko fedarko removed the needs-js-layouts Needs #213 to be taken care of first label Aug 17, 2020
fedarko added a commit to fedarko/empress that referenced this issue Aug 20, 2020
@fedarko
Copy link
Collaborator Author

fedarko commented Aug 20, 2020

A working version of these is ready in this branch, although the code is a bit unpolished:

circlesgobrrr

Currently, bars are approximated by drawing two rectangles (aka four triangles). Although things look smooth for the moving pictures tree, it's clearer what's going on when you use a tiny tree:

tiny

(Ideally we'd draw more rectangles in this case to smooth things out, but it totally works anyway :P)

fedarko added a commit to fedarko/empress that referenced this issue Aug 20, 2020
I would still like to add some tests to the coordinate functions,
but I think this is sufficient to close biocore#297.
kwcantrell pushed a commit that referenced this issue Aug 31, 2020
…gth-scaling (#357)

* ENH: draw circular layout fm barplots! (hacky tho)

* BUG/ENH: correctly set "maxX" in c.bps+use 2 rects

* ENH: make s.m. barplots work in circular layout

how is this actually not that bad what

* DOC: upd8 comment

* DOC: update README re #297

* DOC: document polar -> Cartesian stuff

* DOC/PERF: mark some places to speed up cbarplots

since this kinda uh crashes the EMP tree LMAO

* PERF: reduce unnecessary work in c.barplot drawing

* PERF: chop out more work in c.sm.b. drawing

* MNT: abstract some more c. barplot drawing code

* DOC: document new util funcs for circ barplots

* DOC: clean up inner/outer radius terminology

* Typo fixes

* MNT: max X -> max displacement in bp drawing code

a more elegant way of representing this for circ barplots

* DOC: document precomputed y/angle stuff

* STY: pret

* MNT: shorten nodeAngleInfo var to just angleInfo

* MNT: Catch bad-layout case in _getNodeAngleInfo()

* ENH: Update "barplot unavail" msg re circ barplots

I would still like to add some tests to the coordinate functions,
but I think this is sufficient to close #297.

* PERF: don't draw barplots by dflt (close #343)

* DOC: punctuation

* TST: Test _getNodeAngleInfo()

* TST: add early addCircularBarCoords tst

* TST: finish x-coord circ bar coord test

just gotta test y-coords, with a similar switch stmt

* STY: prettify test

* TST: finish _addCircularBarCoords() test

* TST/MNT: abstract redundant test code

also add approxEqual func which I FEEL LIKE I ALREADY ADDED
but w/e

* TST/DOC: ref toFixedIfy et al in js layout tsts

* MNT: rm check for unsupported layouts in bpdrawing

* DOC: update comments in drawBarplots() start

* TST: Add more context/detail to circ bar comp tst

* DOC: clarify comment

* PERF: don't compute halfAngleRange w rect barplots

Makes things a bit more efficient.

* STY: prettify test error msg

* MNT: abstract rect coord-adding to sep func

* TST: test _addRectangularBarCoords()

* ENH: add length barplot legends; close #354

(still gotta test and document the new code, ofc)

* MNT: Simplify+doc layer drawing code re lenlegends

* TST: Clarify test util funcs

* DOC/TST: refurbish testing utilities docs

* TST: unbreak assignBarplotLengths() tests

* DOC/TST: polish up and test assignBarplotLengths

esp the new functionality. but also i apparently wasn't testing
non-numeric and numeric combos! that's wack. so now that's tested.

* TST: unbreak circ layout js tests

(since nodes are now just referred to by postorder positions)

* DOC: document more length legend stuff

* TST: test Legend.addLengthKey()

* STY: fix improperly named variable - jshint thing

* DOC: fix outdated comment

thanks @ElDeveloper!

* MNT: Simplify maxD computation in drawBarplots()

Thanks @ElDeveloper!

* DOC: fix old node desc in _getNodeAngleInfo()

thanks @kwcantrell!

* DOC: max displacement (not just x) in fm/sm coords

Forgot to update the docs. Thanks @kwcantrell for pointing this out.

Also changed a small block of code to just compute thisLayerMaxD
once and not like 3 times (?? why was I doing that lol)

* MNT: Compute max displacement in getLayoutInfo()

So that this is only done once per layout.

Addresses comment from @kwcantrell.

* STY: add missing semicolon

* TST: restructure approxDeepEqual funcs to use math

... and not stringification :P

Default epsilon is 1e-5, but for a few tests I only bothered writing
things out to 4 decimal places, so for these I use 1e-4.

* TST/STY: clean up utilities for testing

* DOC: tidy up max displacement wording

* BUG: Fix error when searching for invalid name

* DOC: update getNodesWithName() docs

* TST: fix approxDeepEqualMulti docs

thanks @ElDeveloper

* DOC: Update maxDisplacement description

thanks @kwcantrell

* MNT: add .gitattributes file marking vendored code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment