biocore / biocore/empress

Move branch-length-varying stuff into BPTree

Open
#448 1 comment 0 reactions 0 assignees View on GitHub
bug refactoring
Dominant language
JavaScript
Stars
56
Forks
32
PR merge metrics
No merged PRs in 30d

Description

Discussed today re: #444. This would make things like [`this._tree.getTotalLength()`](https://github.com/biocore/empress/blob/dc146db32fe0d1bb655af9b7d14fdc253d641653/empress/support_files/js/empress.js#L2989-L2993) and [`this._tree.length(i)`](https://github.com/biocore/empress/blob/dc146db32fe0d1bb655af9b7d14fdc253d641653/empress/support_files/js/layouts-util.js#L206) manage things depending on whether or not the branch length method is set to `normal`, `ignore`, or `ultrametric`. Would ultimately reduce the amount of work we have to do on keeping track of state.

One of the motivations for this is that there's stuff in `empress.js` for clade collapsing that currently passes the ignore lengths state to the BPTree when computing the "total length" of a clade; it'd be nice to not have to pass this sort of thing to BPTree and just have the class take care of it automatically. Currently, that function doesn't specially handle ultrametric trees, but it all works out because the thing with ultrametric trees is that all tips end at the same distance from the root -- so regardless of which node is chosen as the "deepest" in a clade, it doesn't make a difference when the tree is drawn as ultrametric.

I imagine this would involve updating the BPTree class so that `BPTree.length()` is replaced with two functions: `BPTree.realLength()` and `BPTree.drawnLength()`. The former would return the real length of a node (used for e.g. displaying node lengths in the selection menu), and the latter would handle things based on the `normal` / `ignore` / `ultrametric` stuff.

Contributor guide

Open the contributing guide

Research direction

Start by reading the referenced sections of empress.js and layouts-util.js, then locate the BPTree implementation and inspect how branch-length modes are tracked. Trace callers of getTotalLength() and length(i), including clade collapsing and selection-menu uses. Done means branch-length behavior is managed by BPTree for normal, ignore, and ultrametric modes, with affected callers updated.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
data-visualization
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.