nextstrain / auspice

Web app for visualizing pathogen evolution
https://docs.nextstrain.org/projects/auspice/
GNU Affero General Public License v3.0
292 stars 163 forks source link

draw T branch to parent, not child #200

Open jameshadfield opened 7 years ago

jameshadfield commented 7 years ago

If we define a branch as the stem (the horizontal bit in rectangular view), we currently draw the T (vertical bit in rectangular mode) to the children at the same step. This should be changed so that the T is drawn from a branch to it's parent. This causes issues with T thicknesses and inference of country and is especially apparent on branch dynamic-branch-thickness.

nextstrain.org: image

new nextstrain: image

@rneher thoughts?

rneher commented 7 years ago

Both have advantages and disadvantages. I think the T belongs to the parent and should have the state of the parent as in new nextstrain. Especially in multifurcations, you otherwise have a large number of branches covering each other up. this also messes with the branch tool-tips: they would show mutations of the branch on top even though these mutations are not shared by other children descending from the same multifurcation.

jameshadfield commented 7 years ago

Yes I see. Branch hover-stem-only restricts the hover state to the stem, which would solve some of those issues (if that approach is liked)

The issue @trvrb and I found was in views like this: image

rneher commented 7 years ago

I agree this looks a little odd. but from a logical point of view I like the T way of plotting it.

trvrb commented 7 years ago

@rneher: The situation I really didn't like is shown in the above figure from @jameshadfield. We have the virus moving from purple (French Polynesia) to green (Brazil) and then to red (Haiti). I don't think the bottom part of this branch should be green. It should be red. Like it is in old auspice. Like if the branches were straight lines rather than 90º joins.

d45e4c48-f9ce-11e6-9d62-20c0645bce5e

In FigTree you can change the degree of curvature on branches and go between straight lines and 90º joins. Like so:

rects

smooth

lines

rneher commented 7 years ago

I don't agree. the ancestral virus in your circle was in Brazil. It moved to Haiti at some point along the horizontal part.

trvrb commented 7 years ago

I take your point. We know ancestral node is green and daughter node is red. We know this branch started green and then become red later on. Maybe a gradient would be the most suitable?

rneher commented 7 years ago

I'd suggest we close this for know. we are not going to rewrite this before Tue anyway.

trvrb commented 7 years ago

Can we keep in backlog instead? You were quite convincing on the coloring being okay, but I'm still a little unhappy with the branch thickness behavior. The uniformly fat t-bars give a very different impression than the old style polylines when it comes to line of descent. Compare:

old new

The old version makes it easier to trace the line of descent backwards from selected tips.

rneher commented 7 years ago

sure -- can keep in backlog. despite the coloring. my other gripe is the overlapping of path in multifurcations. On a nicely ladderized tree, the old version looks indeed better. less obvious when things aren't so ladder-like. then you get these funny snake-like patterns. but we can revisit this.