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

Extend usefulness of tip labels #1668

Merged
merged 2 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

* Any `node_attr` in the tree can be used as a tip label, as well as the special-cases of strain-name and "none".
Previously we only allowed valid colorings.
([#1668](https://github.com/nextstrain/auspice/pull/1668))
* The specified tip label is surfaced more prominently within the the on-hover info boxes & on-click modals.
([#1668](https://github.com/nextstrain/auspice/pull/1668))

## version 2.56.1 - 2024/08/22


Expand Down
4 changes: 2 additions & 2 deletions src/actions/recomputeReduxState.js
Original file line number Diff line number Diff line change
Expand Up @@ -555,9 +555,9 @@ const checkAndCorrectErrorsInState = (state, metadata, genomeMap, query, tree, v

/* check tip label is valid. We use the function which generates the options for the dropdown here.
* state.defaults.tipLabelKey is set by the JSON's display_defaults (default: strainSymbol)
* state.tipLabelKey is first set the JSON and then overridden via the URL query (default: state.defaults.tipLabelKey)
* state.tipLabelKey is initially the same value and then overridden via the URL query (default: state.defaults.tipLabelKey)
*/
const validTipLabels = collectAvailableTipLabelOptions(metadata.colorings).map((o) => o.value);
const validTipLabels = collectAvailableTipLabelOptions(tree.nodeAttrKeys, metadata.colorings).map((o) => o.value);
if (!validTipLabels.includes(state.defaults.tipLabelKey)) {
console.error("Invalid JSON-defined tip label:", state.defaults.tipLabelKey);
state.defaults.tipLabelKey = strainSymbol;
Expand Down
23 changes: 13 additions & 10 deletions src/components/controls/choose-tip-label.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import CustomSelect from "./customSelect";

@connect((state) => ({
selected: state.controls.tipLabelKey,
options: collectAvailableTipLabelOptions(state.metadata.colorings)
options: collectAvailableTipLabelOptions(state.tree.nodeAttrKeys, state.metadata.colorings)
}))
class ChooseTipLabel extends React.Component {
constructor(props) {
Expand Down Expand Up @@ -43,11 +43,17 @@ const WithTranslation = withTranslation()(ChooseTipLabel);
export default WithTranslation;

/**
* collect available tip labellings -- currently this is based on the available
* colorings but we ignore genotype (this could be implemented in the future,
* but it's not straightforward)
* available tip labellings are all observed node_attrs as well as two special cases:
* no tip labels / hide & the node name ("sample name"). If a node_attr is a coloring
* then we display the coloring title.
*
* Note that this only considers the main (LHS) tree. It is trivial for this function
* to consider the RHS tree, however the logic in `recomputeReduxState.js` doesn't
* make it straightforward to provide the RHS `nodeAttrKeys` when we construct these.
*
* In the future we could add genotype, but it's not straightforward.
*/
export function collectAvailableTipLabelOptions(colorings) {
export function collectAvailableTipLabelOptions(nodeAttrKeys, colorings) {
return [
/**
* We should consider using a Symbol for the 'none' value so that it
Expand All @@ -56,10 +62,7 @@ export function collectAvailableTipLabelOptions(colorings) {
*/
{value: 'none', label: "none"},
{value: strainSymbol, label: "Sample Name"},
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One oddity I noticed here is the hardcoding of node.name to "Sample Name" which is a little jarring when we use accessions for node.name.

...Object.entries(colorings)
.filter((keyValue) => keyValue[0] !== 'gt' && keyValue[0] !== 'none')
.map(([key, value]) => {
return {value: key, label: value.title};
})
...Array.from(nodeAttrKeys)
.map((key) => ({value: key, label: (colorings[key]||{})?.title||key}))
];
}
22 changes: 11 additions & 11 deletions src/components/tree/infoPanels/click.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import React from "react";
import { isValueValid } from "../../../util/globals";
import { isValueValid, strainSymbol } from "../../../util/globals";
import { infoPanelStyles } from "../../../globalStyles";
import { numericToCalendar } from "../../../util/dateHelpers";
import { getTraitFromNode, getFullAuthorInfoFromNode, getVaccineFromNode,
getAccessionFromNode, getUrlFromNode } from "../../../util/treeMiscHelpers";
import { MutationTable } from "./MutationTable";
import { lhsTreeId} from "../tree";
import { nodeDisplayName } from "./helpers";

export const styles = {
container: {
Expand Down Expand Up @@ -241,9 +242,10 @@ const Trait = ({node, trait, colorings, isTerminal}) => {
* @param {Object} props.colorings
* @param {Object} props.observedMutations
* @param {function} props.geneSortFn
* @param {string|symbol} props.tipLabelKey
* @param {function} props.t
*/
const NodeClickedPanel = ({selectedNode, nodesLhsTree, nodesRhsTree, clearSelectedNode, colorings, observedMutations, geneSortFn, t}) => {
const NodeClickedPanel = ({selectedNode, nodesLhsTree, nodesRhsTree, clearSelectedNode, colorings, observedMutations, geneSortFn, tipLabelKey, t}) => {
if (!selectedNode) return null;
const node = (selectedNode.treeId===lhsTreeId ? nodesLhsTree : nodesRhsTree)?.[selectedNode.idx];
if (!node) {
Expand All @@ -252,25 +254,23 @@ const NodeClickedPanel = ({selectedNode, nodesLhsTree, nodesRhsTree, clearSelect
}
const panelStyle = { ...infoPanelStyles.panel};
panelStyle.maxHeight = "70%";

/* We have `isTerminal` and `isTip` to differentiate between clicking on a branch leading to a tip
* vs clicking on the tip (circle) itself */
const isTerminal = !node.hasChildren;
const isTip = !selectedNode.isBranch;

const title = isTip ?
node.name :
isTerminal ?
`Branch leading to ${node.name}` :
"Internal branch";
const shouldShowNodeName = tipLabelKey!==strainSymbol;

return (
<div style={infoPanelStyles.modalContainer} onClick={() => clearSelectedNode(selectedNode)}>
<div className={"panel"} style={panelStyle} onClick={(e) => stopProp(e)}>
<StrainName>{title}</StrainName>
<StrainName>{nodeDisplayName(t, node, tipLabelKey, !isTip)}</StrainName>
<table>
<tbody>
{!isTip && item(t("Number of terminal tips"), node.fullTipCount)}
{!isTerminal && item(t("Number of terminal tips"), node.fullTipCount)}
{shouldShowNodeName && item(t("Node name"), node.name)}
{isTip && <VaccineInfo node={node} t={t}/>}
<SampleDate isTerminal={isTerminal} node={node} t={t}/>
{!isTip && item("Node name", node.name)}
{isTip && <PublicationInfo node={node} t={t}/>}
{getTraitsToDisplay(node).map((trait) => (
<Trait node={node} trait={trait} colorings={colorings} key={trait} isTerminal={isTerminal}/>
Expand Down
23 changes: 23 additions & 0 deletions src/components/tree/infoPanels/helpers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { getTraitFromNode } from "../../../util/treeMiscHelpers";
import { numericToCalendar } from "../../../util/dateHelpers";


/**
* Attempt to display the best name we can for a node, depending on how we are looking at a node.
* The returned string will be rendered on a line of its own, so an empty string will look ok
* Future enhancement: we could examine the coloring metadata (if available) and format the value accordingly
*/
export function nodeDisplayName(t, node, tipLabelKey, branch) {
let tipLabel = getTraitFromNode(node, tipLabelKey)
if (tipLabelKey==='num_date' && tipLabel) tipLabel = numericToCalendar(tipLabel)
const terminal = !node.hasChildren;

if (branch) {
if (terminal) {
return tipLabel ? t("Branch leading to {{tipLabel}}", {tipLabel}) : t("Terminal branch") // hover + click
}
return t("Internal branch") // branch click only
}
/* TIP */
return tipLabel;
}
23 changes: 10 additions & 13 deletions src/components/tree/infoPanels/hover.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@ import { getTipColorAttribute } from "../../../util/colorHelpers";
import { isColorByGenotype, decodeColorByGenotype } from "../../../util/getGenotype";
import { getTraitFromNode, getDivFromNode, getVaccineFromNode,
getFullAuthorInfoFromNode, getTipChanges, getBranchMutations } from "../../../util/treeMiscHelpers";
import { isValueValid } from "../../../util/globals";
import { isValueValid, strainSymbol } from "../../../util/globals";
import { formatDivergence, getIdxOfInViewRootNode } from "../phyloTree/helpers";
import { parseIntervalsOfNsOrGaps } from "./MutationTable";
import { nodeDisplayName } from "./helpers";

export const InfoLine = ({name, value, padBelow=false}) => {
const renderValues = () => {
Expand All @@ -32,12 +33,6 @@ export const InfoLine = ({name, value, padBelow=false}) => {
);
};

const StrainName = ({name}) => (
<div style={infoPanelStyles.tooltipHeading}>
{name}
</div>
);

/**
* A React component to display information about the branch's time & divergence (where applicable)
* @param {Object} props
Expand Down Expand Up @@ -267,14 +262,13 @@ const BranchMutations = ({node, geneSortFn, observedMutations, t}) => {
* @param {Object} props
* @param {Object} props.node branch node which is currently highlighted
*/
const BranchDescendents = ({node, t}) => {
const BranchDescendants = ({node, t, tipLabelKey}) => {
const [name, value] = node.fullTipCount === 1 ?
[t("Branch leading to"), node.name] :
[nodeDisplayName(t, node, tipLabelKey, true), ""] :
[t("Number of descendants")+":", node.fullTipCount];
return <InfoLine name={name} value={value} padBelow/>;
};


/**
* A React component to show vaccine information, if present
* @param {Object} props
Expand Down Expand Up @@ -399,17 +393,20 @@ const HoverInfoPanel = ({
colorings,
geneSortFn,
observedMutations,
tipLabelKey,
t
}) => {
if (!selectedNode) return null
const node = selectedNode.node.n; // want the redux node, not the phylo node
const idxOfInViewRootNode = getIdxOfInViewRootNode(node);

return (
<Container node={node} panelDims={panelDims}>
{selectedNode.isBranch===false ? (
<>
<StrainName name={node.name}/>
<div style={infoPanelStyles.tooltipHeading}>
{nodeDisplayName(t, node, tipLabelKey, false)}
</div>
{tipLabelKey!==strainSymbol && <InfoLine name="Node name:" value={node.name}/>}
<VaccineInfo node={node} t={t}/>
<TipMutations node={node} t={t}/>
<BranchLength node={node} t={t}/>
Expand All @@ -419,7 +416,7 @@ const HoverInfoPanel = ({
</>
) : (
<>
<BranchDescendents node={node} t={t}/>
<BranchDescendants node={node} t={t} tipLabelKey={tipLabelKey}/>
<BranchMutations node={node} geneSortFn={geneSortFn} observedMutations={observedMutations} t={t}/>
<BranchLength node={node} t={t}/>
<ColorBy node={node} colorBy={colorBy} colorByConfidence={colorByConfidence} colorScale={colorScale} colorings={colorings}/>
Expand Down
2 changes: 2 additions & 0 deletions src/components/tree/tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ class Tree extends React.Component {
geneSortFn={this.state.geneSortFn}
observedMutations={this.props.tree.observedMutations}
panelDims={{width: this.props.width, height: this.props.height, spaceBetweenTrees}}
tipLabelKey={this.props.tipLabelKey}
t={t}
/>
<NodeClickedPanel
Expand All @@ -216,6 +217,7 @@ class Tree extends React.Component {
observedMutations={this.props.tree.observedMutations}
colorings={this.props.colorings}
geneSortFn={this.state.geneSortFn}
tipLabelKey={this.props.tipLabelKey}
t={t}
/>
{this.props.showTangle && this.state.tree && this.state.treeToo ? (
Expand Down
19 changes: 15 additions & 4 deletions src/util/treeJsonProcessing.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,15 @@ const pseudoRandomName = () => (Math.random()*1e32).toString(36).slice(0, 6);
* node.hasChildren {bool}
* node.arrayIdx {integer} - the index of the node in the nodes array
* @param {array} nodes redux tree nodes
* @return {array} input array (kinda unnecessary)
* @return {Object} ret
* @return {Set} ret.nodeAttrKeys collection of all `node_attr` keys whose values are Objects
* @return {Array} ret.nodes input array (kinda unnecessary)
*
* side-effects: node.hasChildren (bool) and node.arrayIdx (INT) for each node in nodes
*/
const processNodes = (nodes) => {
const nodeNamesSeen = new Set();
const nodeAttrKeys = new Set();
calcFullTipCounts(nodes[0]); /* recursive. Uses d.children */
nodes.forEach((d, idx) => {
d.arrayIdx = idx; /* set an index so that we can access visibility / nodeColors if needed */
Expand All @@ -32,8 +36,15 @@ const processNodes = (nodes) => {
console.warn(`Tree node detected with a duplicate name. Changing '${prev}' to '${d.name}' and continuing...`);
}
nodeNamesSeen.add(d.name);

for (const [attrKey, attrValue] of Object.entries(d.node_attrs || {})) {
if (typeof attrValue === 'object' && 'value' in attrValue) {
nodeAttrKeys.add(attrKey)
}
}
Comment on lines +40 to +44
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-blocking, probably more of a change in Augur

I was looking at the augur export schema to figure out which attrs this would be included here. Worth noting that we would not be able to use accession as the tip label because it does not use the value key.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's a good point. I wish we had made the schema so that every node-attr was the same structure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to refrain from addressing this here, but I do think moving all node_attrs values to objects will reduce complexity in Auspice a lot, see for example all these different getters. We don't have to do this on the augur side (necessitating schema changes and all that), we could probably do it within treeJsonProcessing.js.


});
return nodes;
return {nodeAttrKeys, nodes};
};

/**
Expand Down Expand Up @@ -165,7 +176,7 @@ export const treeJsonToState = (treeJSON) => {
nodesArray.push(...flattenTree(treeRootNode));
}
nodesArray.unshift(makeSubtreeRootNode(nodesArray, subtreeIndicies));
const nodes = processNodes(nodesArray);
const {nodeAttrKeys, nodes} = processNodes(nodesArray);
addParentInfo(nodesArray);
const vaccines = nodes.filter((d) => {
const v = getVaccineFromNode(d);
Expand All @@ -174,6 +185,6 @@ export const treeJsonToState = (treeJSON) => {
const availableBranchLabels = processBranchLabelsInPlace(nodesArray);
const observedMutations = collectObservedMutations(nodesArray);
return Object.assign({}, getDefaultTreeState(), {
nodes, vaccines, observedMutations, availableBranchLabels, loaded: true
nodes, nodeAttrKeys, vaccines, observedMutations, availableBranchLabels, loaded: true
});
};
Loading