Remove duplicate property storage

We represent tree nodes by two different objects, a "PhyloNode" and a "Node", although these names are not formalised. The former holds information necessary to render the phylogenetic tree and is solely used by `PhyloTree` while the latter holds more general information. They are linked: `phyloNode = node.n` and `node = phyloNode.shell`.

This commit tackles deeply embedded technical debt where each object held a copy of the same data. Specifically,
* `children` is now only stored on a Node
* `parent` is similarly stored on a Node
* `terminal` is removed from PhyloNode (we can refer to node.hasChildren)

There is (at least) one remaining duplicated property - `inView` - which is set on <Node> when the dataset loads due to the corresponding <PhyloNode> not yet existing. This is left for future work as the concept of `inView` may change as we develop further methods for tree zoom / pan.

src/actions/recomputeReduxState.js

@@ -589,10 +589,9 @@ const checkAndCorrectErrorsInState = (state, metadata, query, tree, viewingNarra
 };
 
 const modifyTreeStateVisAndBranchThickness = (oldState, zoomSelected, controlsState, dispatch) => {
-  /* calculate new branch thicknesses & visibility */
+  /* calculate the index of the (in-view) root note, which depends on any selected zoom */
   let newIdxRoot = oldState.idxOfInViewRootNode;
   if (zoomSelected) {
-    // Check and fix old format 'clade=B' - in this case selectionClade is just 'B'
     const [labelName, labelValue] = zoomSelected.split(":");
     const cladeSelectedIdx = getIdxMatchingLabel(oldState.nodes, labelName, labelValue, dispatch);
     oldState.selectedClade = zoomSelected;
@@ -601,6 +600,8 @@ const modifyTreeStateVisAndBranchThickness = (oldState, zoomSelected, controlsSt
     oldState.selectedClade = undefined;
     newIdxRoot = applyInViewNodesToTree(0, oldState);
   }
+
+  /* calculate new branch thickesses & visibility, as this depends on the root note */
   const visAndThicknessData = calculateVisiblityAndBranchThickness(
     oldState,
     controlsState,

src/actions/tree.js

@@ -9,28 +9,41 @@ import { constructVisibleTipLookupBetweenTrees } from "../util/treeTangleHelpers
 import { createVisibleLegendValues } from "../util/colorScale";
 
 
+/**
+ * Updates the `inView` property of nodes which depends on the currently selected
+ * root index (i.e. what node the tree is zoomed into).
+ * Note that this property is historically the remit of PhyloTree, however this function
+ * may be called before those objects are created; in this case we store the property on
+ * the tree node itself.
+ * @param {Int} idx - index of displayed root node
+ * @param {ReduxTreeState} tree
+ */
 export const applyInViewNodesToTree = (idx, tree) => {
   const validIdxRoot = idx !== undefined ? idx : tree.idxOfInViewRootNode;
-  if (idx !== tree.idxOfInViewRootNode && tree.nodes[0].shell) {
-    /* a bit hacky, should be somewhere else */
+  if (tree.nodes[0].shell) {
     tree.nodes.forEach((d) => {
       d.shell.inView = false;
       d.shell.update = true;
     });
-    if (tree.nodes[validIdxRoot].shell.terminal) {
-      applyToChildren(tree.nodes[validIdxRoot].shell.parent, (d) => {d.inView = true;});
-    } else {
+    if (tree.nodes[validIdxRoot].hasChildren) {
       applyToChildren(tree.nodes[validIdxRoot].shell, (d) => {d.inView = true;});
+    } else {
+      applyToChildren(tree.nodes[validIdxRoot].parent.shell, (d) => {d.inView = true;});
     }
   } else {
+    /* FYI applyInViewNodesToTree is now setting inView on the redux nodes */
     tree.nodes.forEach((d) => {
       d.inView = false;
     });
-    if (!tree.nodes[validIdxRoot].hasChildren) {
-      applyToChildren(tree.nodes[validIdxRoot].parent, (d) => {d.inView = true;});
-    } else {
-      applyToChildren(tree.nodes[validIdxRoot], (d) => {d.inView = true;});
-    }
+    /* note that we cannot use `applyToChildren` as that operates on PhyloNodes */
+    const _markChildrenInView = (node) => {
+      node.inView = true;
+      if (node.children) {
+        for (const child of node.children) _markChildrenInView(child);
+      }
+    };
+    const startingNode = tree.nodes[validIdxRoot].hasChildren ? tree.nodes[validIdxRoot] : tree.nodes[validIdxRoot].parent;
+    _markChildrenInView(startingNode);
   }
 
   return validIdxRoot;

src/components/tree/phyloTree/change.js

@@ -335,7 +335,9 @@ export const change = function change({
       d.update = true;
     });
     /* if clade is terminal, use the parent as the zoom node */
-    this.zoomNode = zoomIntoClade.terminal ? zoomIntoClade.parent : zoomIntoClade;
+    this.zoomNode = zoomIntoClade.n.hasChildren ?
+      zoomIntoClade :
+      zoomIntoClade.n.parent.shell;
     applyToChildren(this.zoomNode, (d) => {d.inView = true;});
   }
   if (svgHasChangedDimensions) {

src/components/tree/phyloTree/helpers.js

@@ -15,81 +15,62 @@ export const getDomId = (type, strain) => {
  * computes a measure of the total number of leaves for each node in
  * the tree, weighting leaves differently if they are inView.
  * Note: function is recursive
- * @param {obj} node -- root node of the tree
+ * @param {PhyloNode} node -- root node of the tree
  * @returns {undefined}
  * @sideEffects sets `node.leafCount` {number} for all nodes
  */
 export const addLeafCount = (node) => {
-  if (node.terminal && node.inView) {
+  const terminal = !node.n.hasChildren;
+  if (terminal && node.inView) {
     node.leafCount = 1;
-  } else if (node.terminal && !node.inView) {
+  } else if (terminal && !node.inView) {
     node.leafCount = 0.15;
   } else {
     node.leafCount = 0;
-    for (let i = 0; i < node.children.length; i++) {
-      addLeafCount(node.children[i]);
-      node.leafCount += node.children[i].leafCount;
+    for (const child of node.n.children) {
+      const phyloChild = child.shell;
+      addLeafCount(phyloChild);
+      node.leafCount += phyloChild.leafCount;
     }
   }
 };
 
 
-/*
+/**
  * this function takes a call back and applies it recursively
  * to all child nodes, including internal nodes
- * @params:
- *   node -- node to whose children the function is to be applied
- *   func -- call back function to apply
+ * @param {PhyloNode} node
+ * @param {Function} func - function to apply to each children. Is passed a single argument, the <PhyloNode> of the children.
  */
-export const applyToChildren = (node, func) => {
-  func(node);
-  if (node.terminal || node.children === undefined) { // in case clade set by URL, terminal hasn't been set yet!
+export const applyToChildren = (phyloNode, func) => {
+  func(phyloNode);
+  const node = phyloNode.n;
+  if ((!node.hasChildren) || (node.children === undefined)) { // in case clade set by URL, terminal hasn't been set yet!
     return;
   }
-  for (let i = 0; i < node.children.length; i++) {
-    applyToChildren(node.children[i], func);
+  for (const child of node.children) {
+    applyToChildren(child.shell, func);
   }
 };
 
-
-/*
-* given nodes, create the children and parent properties.
-* modifies the nodes argument in place
-*/
-export const createChildrenAndParentsReturnNumTips = (nodes) => {
-  let numTips = 0;
-  nodes.forEach((d) => {
-    d.parent = d.n.parent.shell;
-    if (d.terminal) {
-      d.orderRange = [d.displayOrder, d.displayOrder];
-      d.children = null;
-      numTips++;
-    } else {
-      d.orderRange = [d.n.children[0].shell.displayOrder, d.n.children[d.n.children.length - 1].shell.displayOrder];
-      d.children = [];
-      for (let i = 0; i < d.n.children.length; i++) {
-        d.children.push(d.n.children[i].shell);
-      }
-    }
-  });
-  return numTips;
-};
-
 /** setDisplayOrderRecursively
+ * @param {PhyloNode} node
+ * @param {int} yCounter
  */
 export const setDisplayOrderRecursively = (node, yCounter) => {
-  if (node.children) {
-    for (let i = node.children.length - 1; i >= 0; i--) {
-      yCounter = setDisplayOrderRecursively(node.children[i], yCounter);
+  const children = node.n.children; // (redux) tree node
+  if (children) {
+    for (let i = children.length - 1; i >= 0; i--) {
+      yCounter = setDisplayOrderRecursively(children[i].shell, yCounter);
     }
   } else {
     node.displayOrder = ++yCounter;
     node.displayOrderRange = [yCounter, yCounter];
     return yCounter;
   }
   /* if here, then all children have displayOrders, but we dont. */
-  node.displayOrder = node.children.reduce((acc, d) => acc + d.displayOrder, 0) / node.children.length;
-  node.displayOrderRange = [node.n.children[0].shell.displayOrder, node.n.children[node.n.children.length - 1].shell.displayOrder];
+  node.displayOrder = children.reduce((acc, d) => acc + d.shell.displayOrder, 0) / children.length;
+  node.displayOrderRange = [children[0].shell.displayOrder, children[children.length - 1].shell.displayOrder];
   return yCounter;
 };
 
@@ -100,6 +81,7 @@ export const setDisplayOrderRecursively = (node, yCounter) => {
  * PhyloTree can subsequently use this information. Accessed by prototypes
  * rectangularLayout, radialLayout, createChildrenAndParents
  * side effects: <phyloNode>.displayOrder (i.e. in the redux node) and <phyloNode>.displayOrderRange
+ * @param {Array<PhyloNode>} nodes
  */
 export const setDisplayOrder = (nodes) => setDisplayOrderRecursively(nodes[0], 0);
 

src/components/tree/phyloTree/labels.js

@@ -14,7 +14,7 @@ export const updateTipLabels = function updateTipLabels(dt) {
   const xPad = this.params.tipLabelPadX;
   const yPad = this.params.tipLabelPadY;
 
-  const inViewTips = this.nodes.filter((d) => d.terminal).filter((d) => d.inView);
+  const inViewTips = this.nodes.filter((d) => !d.n.hasChildren).filter((d) => d.inView);
 
   const inViewVisibleTips = inViewTips.filter((d) => d.visibility === NODE_VISIBLE);
 

src/components/tree/phyloTree/layouts.js

@@ -123,20 +123,19 @@ export const scatterplotLayout = function scatterplotLayout() {
 
 };
 
-/*
+/**
  * Utility function for the unrooted tree layout.
  * assigns x,y coordinates to the subtree starting in node
- * @params:
- *   node -- root of the subtree.
- *   nTips -- total number of tips in the tree.
+ * @param {PhyloNode} node
+ * @param {Int} nTips - total number of tips in the tree.
  */
 const unrootedPlaceSubtree = (node, nTips) => {
   node.x = node.px + node.branchLength * Math.cos(node.tau + node.w * 0.5);
   node.y = node.py + node.branchLength * Math.sin(node.tau + node.w * 0.5);
   let eta = node.tau; // eta is the cumulative angle for the wedges in the layout
-  if (!node.terminal) {
-    for (let i = 0; i < node.children.length; i++) {
-      const ch = node.children[i];
+  if (node.n.hasChildren) {
+    for (let i = 0; i < node.n.children.length; i++) {
+      const ch = node.n.children[i].shell; // ch is a <PhyloNode>
       ch.w = 2 * Math.PI * ch.leafCount / nTips;
       ch.tau = eta;
       eta += ch.w;
@@ -169,13 +168,15 @@ export const unrootedLayout = function unrootedLayout() {
   this.nodes[0].w = 2 * Math.PI;
   this.nodes[0].tau = 0;
   let eta = 1.5 * Math.PI;
-  for (let i = 0; i < this.nodes[0].children.length; i++) {
-    this.nodes[0].children[i].px = 0;
-    this.nodes[0].children[i].py = 0;
-    this.nodes[0].children[i].w = 2.0 * Math.PI * this.nodes[0].children[i].leafCount / nTips;
-    this.nodes[0].children[i].tau = eta;
-    eta += this.nodes[0].children[i].w;
-    unrootedPlaceSubtree(this.nodes[0].children[i], nTips);
+  const children = this.nodes[0].n.children; // <Node> not <PhyloNode>
+  for (let i = 0; i < children.length; i++) {
+    const phyloNode = children[i].shell;
+    phyloNode.px = 0;
+    phyloNode.py = 0;
+    phyloNode.w = 2.0 * Math.PI * phyloNode.leafCount / nTips;
+    phyloNode.tau = eta;
+    eta += phyloNode.w;
+    unrootedPlaceSubtree(phyloNode, nTips);
   }
   if (this.vaccines) {
     this.vaccines.forEach((d) => {
@@ -319,7 +320,7 @@ export const setScales = function setScales() {
 export const mapToScreen = function mapToScreen() {
   timerStart("mapToScreen");
 
-  const inViewTerminalNodes = this.nodes.filter((d) => d.terminal).filter((d) => d.inView);
+  const inViewTerminalNodes = this.nodes.filter((d) => !d.n.hasChildren).filter((d) => d.inView);
 
   /* set up space (padding) for axes etc, as we don't want the branches & tips to occupy the entire SVG! */
   this.margins = {
@@ -336,7 +337,7 @@ export const mapToScreen = function mapToScreen() {
   // scatterplots further restrict nodes used for domain calcs - if not rendering branches,
   // then we don't consider internal nodes for the domain calc
   if (this.layout==="scatter" && this.scatterVariables.showBranches===false) {
-    nodesInDomain = nodesInDomain.filter((d) => d.terminal);
+    nodesInDomain = nodesInDomain.filter((d) => !d.n.hasChildren);
   }
 
   /* Compute the domains to pass to the d3 scales for the x & y axes */
@@ -457,23 +458,23 @@ export const mapToScreen = function mapToScreen() {
       this.nodes.forEach((d) => {d.branch=[];});
     }
   } else if (this.layout==="rect") {
-    this.nodes.forEach((d) => {
-      const stem_offset = 0.5*(d.parent["stroke-width"] - d["stroke-width"]) || 0.0;
-      const childrenY = [this.yScale(d.displayOrderRange[0]), this.yScale(d.displayOrderRange[1])];
+    this.nodes.forEach((d) => { // d is a <PhyloNode>
+      const stem_offset = 0.5*(d.n.parent.shell["stroke-width"] - d["stroke-width"]) || 0.0;
+      const stemRange = [this.yScale(d.displayOrderRange[0]), this.yScale(d.displayOrderRange[1])];
       // Note that a branch cannot be perfectly horizontal and also have a (linear) gradient applied to it
       // So we add a tiny amount of jitter (e.g 1/1000px) to the horizontal line (d.branch[0])
       // see https://stackoverflow.com/questions/13223636/svg-gradient-for-perfectly-horizontal-path
       d.branch =[
         [` M ${d.xBase - stem_offset},${d.yBase} L ${d.xTip},${d.yTip+0.01}`],
-        [` M ${d.xTip},${childrenY[0]} L ${d.xTip},${childrenY[1]}`]
+        [` M ${d.xTip},${stemRange[0]} L ${d.xTip},${stemRange[1]}`]
       ];
       if (this.params.confidence) {
         d.confLine =` M ${this.xScale(d.conf[0])},${d.yBase} L ${this.xScale(d.conf[1])},${d.yTip}`;
       }
     });
   } else if (this.layout==="radial") {
     const offset = this.nodes[0].depth;
-    const stem_offset_radial = this.nodes.map((d) => {return (0.5*(d.parent["stroke-width"] - d["stroke-width"]) || 0.0);});
+    const stem_offset_radial = this.nodes.map((d) => {return (0.5*(d.n.parent.shell["stroke-width"] - d["stroke-width"]) || 0.0);});
     this.nodes.forEach((d) => {d.cBarStart = this.yScale(d.displayOrderRange[0]);});
     this.nodes.forEach((d) => {d.cBarEnd = this.yScale(d.displayOrderRange[1]);});
     this.nodes.forEach((d, i) => {
@@ -482,7 +483,7 @@ export const mapToScreen = function mapToScreen() {
         " "+(d.yBase-stem_offset_radial[i]*Math.cos(d.angle)).toString() +
         " L "+d.xTip.toString()+" "+d.yTip.toString(), ""
       ];
-      if (!d.terminal) {
+      if (d.n.hasChildren) {
         d.branch[1] =[" M "+this.xScale(d.xCBarStart).toString()+" "+this.yScale(d.yCBarStart).toString()+
         " A "+(this.xScale(d.depth)-this.xScale(offset)).toString()+" "+
         (this.yScale(d.depth)-this.yScale(offset)).toString()+
@@ -516,12 +517,12 @@ function jitter(axis, scale, nodes) {
   }
   const [base, tip, randLen] = [`${axis}Base`, `${axis}Tip`, rand.length];
   let j = 0;
-  function recurse(n) {
-    n[base] = n.parent[tip];
-    n[tip] += rand[j++];
+  function recurse(phyloNode) {
+    phyloNode[base] = phyloNode.n.parent.shell[tip];
+    phyloNode[tip] += rand[j++];
     if (j>=randLen) j=0;
-    if (!n.children) return;
-    for (const child of n.children) recurse(child);
+    if (!phyloNode.n.hasChildren) return;
+    for (const child of phyloNode.n.children) recurse(child.shell);
   }
   recurse(nodes[0]);
 }

src/components/tree/phyloTree/phyloTree.js

@@ -1,5 +1,4 @@
 import { createDefaultParams } from "./defaultParams";
-import { createChildrenAndParentsReturnNumTips } from "./helpers";
 import { change, modifySVG, modifySVGInStages } from "./change";
 
 /* PROTOTYPES */
@@ -29,13 +28,12 @@ const PhyloTree = function PhyloTree(reduxNodes, id, idxOfInViewRootNode) {
       n: d, /* a back link to the redux node */
       x: 0,
       y: 0,
-      terminal: (typeof d.children === "undefined"),
       inView: d.inView !== undefined ? d.inView : true /* each node is visible, unless set earlier! */
     };
     d.shell = phyloNode; /* set the link from the redux node to the phylotree node */
     return phyloNode;
   });
-  this.numberOfTips = createChildrenAndParentsReturnNumTips(this.nodes);
+  this.numberOfTips = reduxNodes.filter((n) => !n.hasChildren).length;
   this.zoomNode = this.nodes[idxOfInViewRootNode];
   this.strainToNode = {};
   this.nodes.forEach((phylonode) => {this.strainToNode[phylonode.n.name] = phylonode;});

src/components/tree/phyloTree/regression.js

@@ -9,7 +9,7 @@ import { formatDivergence, guessAreMutationsPerSite} from "./helpers";
  * It does not consider which tips are inView / visible.
  */
 export function calculateRegressionThroughRoot(nodes) {
-  const terminalNodes = nodes.filter((d) => d.terminal);
+  const terminalNodes = nodes.filter((d) => !d.n.hasChildren);
   const nTips = terminalNodes.length;
   const offset = nodes[0].x;
   const XY = sum(
@@ -30,7 +30,7 @@ export function calculateRegressionThroughRoot(nodes) {
  * This function does not consider which tips are inView / visible.
  */
 export function calculateRegressionWithFreeIntercept(nodes) {
-  const terminalNodesWithXY = nodes.filter((d) => d.terminal && d.x!==undefined && d.y!==undefined);
+  const terminalNodesWithXY = nodes.filter((d) => (!d.n.hasChildren) && d.x!==undefined && d.y!==undefined);
   const nTips = terminalNodesWithXY.length;
   const meanX = sum(terminalNodesWithXY.map((d) => d.x))/nTips;
   const meanY = sum(terminalNodesWithXY.map((d) => d.y))/nTips;