From bc4720133e77875df690ecade104acfc492d643d Mon Sep 17 00:00:00 2001 From: james hadfield Date: Tue, 11 Jun 2024 17:23:08 +1200 Subject: [PATCH 1/5] Unify common code for loading a second tree There are two ways of loading a second tree: upon initial load / narrative slide progression (logic within `createStateFromQueryOrJSONs`) or via the sidebar "second tree" UI (logic within `createTreeTooState`). This commit aims to reduce the duplication of code between these functions, where possible given the current design. There is one behavioural change - the second method of loading a tree now no longer skips the `castIncorrectTypes` error correction function. Tangentially: the code in this file is fragile and the order of function calls is crucial. I hope the adoption of TS will improve resilience here. --- src/actions/recomputeReduxState.js | 106 ++++++++++++++--------------- 1 file changed, 51 insertions(+), 55 deletions(-) diff --git a/src/actions/recomputeReduxState.js b/src/actions/recomputeReduxState.js index a7e6e149b..c8d641222 100644 --- a/src/actions/recomputeReduxState.js +++ b/src/actions/recomputeReduxState.js @@ -690,26 +690,6 @@ const modifyTreeStateVisAndBranchThickness = (oldState, zoomSelected, controlsSt return newState; }; -const removePanelIfPossible = (panels, name) => { - const idx = panels.indexOf(name); - if (idx !== -1) { - panels.splice(idx, 1); - } -}; - -const modifyControlsViaTreeToo = (controls, name) => { - controls.showTreeToo = name; - controls.showTangle = true; - controls.layout = "rect"; /* must be rectangular for two trees */ - controls.panelsToDisplay = controls.panelsToDisplay.slice(); - removePanelIfPossible(controls.panelsToDisplay, "map"); - removePanelIfPossible(controls.panelsToDisplay, "entropy"); - removePanelIfPossible(controls.panelsToDisplay, "frequencies"); - controls.canTogglePanelLayout = false; - controls.panelLayout = "full"; - return controls; -}; - /** * The v2 JSON spec defines colorings as a list, so that order is guaranteed. * Prior to this, we used a dict, where key insertion order is (guaranteed? essentially always?) @@ -877,14 +857,7 @@ export const createStateFromQueryOrJSONs = ({ tree.name = mainTreeName; metadata.mainTreeNumTips = calcTotalTipsInTree(tree.nodes); if (secondTreeDataset) { - treeToo = treeJsonToState(secondTreeDataset.tree); - castIncorrectTypes(metadata, treeToo); - treeToo.debug = "RIGHT"; - treeToo.name = secondTreeName; - updateMetadataStateViaSecondTree(metadata, secondTreeDataset, entropy?.genomeMap) - - /* TODO: calc & display num tips in 2nd tree */ - // metadata.secondTreeNumTips = calcTotalTipsInTree(treeToo.nodes); + ({treeToo, metadata} = instantiateSecondTree(secondTreeDataset, metadata, entropy?.genomeMap, secondTreeName)); } /* new controls state - don't apply query yet (or error check!) */ @@ -965,11 +938,7 @@ export const createStateFromQueryOrJSONs = ({ tree = modifyTreeStateVisAndBranchThickness(tree, query.label, controls, dispatch); if (treeToo && treeToo.loaded) { - treeToo.nodeColorsVersion = tree.nodeColorsVersion; - treeToo.nodeColors = calcNodeColor(treeToo, controls.colorScale); - treeToo = modifyTreeStateVisAndBranchThickness(treeToo, undefined, controls, dispatch); - controls = modifyControlsViaTreeToo(controls, treeToo.name); - treeToo.tangleTipLookup = constructVisibleTipLookupBetweenTrees(tree.nodes, treeToo.nodes, tree.visibility, treeToo.visibility); + treeToo = updateSecondTree(tree, treeToo, controls, dispatch) } /* we can only calculate which legend items we wish to display _after_ the visibility has been calculated */ @@ -1044,33 +1013,60 @@ export const createTreeTooState = ({ /* TODO: reconcile choices (filters, colorBys etc) with this new tree */ /* TODO: reconcile query with visibility etc */ - const metadata = {...oldState.metadata}; - updateMetadataStateViaSecondTree(metadata, json, oldState.entropy?.genomeMap); - - let controls = {...oldState.controls}; const tree = Object.assign({}, oldState.tree); tree.name = originalTreeUrl; - let treeToo = treeJsonToState(json.tree); - treeToo.name = secondTreeUrl; - treeToo.debug = "RIGHT"; - controls = modifyControlsStateViaTree(controls, tree, treeToo, oldState.metadata.colorings); - controls = modifyControlsViaTreeToo(controls, secondTreeUrl); - treeToo = modifyTreeStateVisAndBranchThickness(treeToo, undefined, controls, dispatch); + let {treeToo, metadata} = instantiateSecondTree(json, oldState.metadata, oldState.entropy?.genomeMap, secondTreeUrl) - /* calculate colours if loading from JSONs or if the query demands change */ - const colorScale = calcColorScale(controls.colorBy, controls, tree, treeToo, metadata); - const nodeColors = calcNodeColor(treeToo, colorScale); - tree.nodeColors = calcNodeColor(tree, colorScale); // also update main tree's colours - tree.nodeColorsVersion++; - - controls.colorScale = colorScale; + /* recompute the controls state now that we have new data */ + const controls = modifyControlsStateViaTree({...oldState.controls}, tree, treeToo, oldState.metadata.colorings); + /* recalculate the color scale with updated tree data */ + controls.colorScale = calcColorScale(controls.colorBy, controls, tree, treeToo, metadata); controls.colorByConfidence = doesColorByHaveConfidence(controls, controls.colorBy); - treeToo.nodeColorsVersion = colorScale.version; - treeToo.nodeColors = nodeColors; + /* and update the color scale as applied to the LHS tree */ + tree.nodeColors = calcNodeColor(tree, controls.colorScale); + tree.nodeColorsVersion++; - treeToo.tangleTipLookup = constructVisibleTipLookupBetweenTrees( - tree.nodes, treeToo.nodes, tree.visibility, treeToo.visibility - ); + treeToo = updateSecondTree(tree, treeToo, controls, dispatch); return {tree, treeToo, controls, metadata}; }; + + +/** + * Code which is common to both loading a second tree within `createStateFromQueryOrJSONs` and + * `createTreeTooState`. + */ +function instantiateSecondTree(secondTreeDataset, metadata, genomeMap, secondTreeName) { + const treeToo = treeJsonToState(secondTreeDataset.tree); + castIncorrectTypes(metadata, treeToo); + treeToo.debug = "RIGHT"; + treeToo.name = secondTreeName; + updateMetadataStateViaSecondTree({...metadata}, secondTreeDataset, genomeMap) + + /* TODO: calc & display num tips in 2nd tree */ + // metadata.secondTreeNumTips = calcTotalTipsInTree(treeToo.nodes); + + return {treeToo, metadata} +} + +/** + * Update colours and control state options. This function requires that the controls state + * has been instantiated (e.g. the colorScale has been computed) + */ +function updateSecondTree(tree, treeToo, controls, dispatch) { + treeToo.nodeColorsVersion = tree.nodeColorsVersion; + treeToo.nodeColors = calcNodeColor(treeToo, controls.colorScale); + treeToo = modifyTreeStateVisAndBranchThickness(treeToo, undefined, controls, dispatch); + treeToo.tangleTipLookup = constructVisibleTipLookupBetweenTrees(tree.nodes, treeToo.nodes, tree.visibility, treeToo.visibility); + + /* modify controls */ + controls.showTreeToo = treeToo.name; + controls.showTangle = true; + controls.layout = "rect"; /* must be rectangular for two trees */ + controls.panelsToDisplay = controls.panelsToDisplay + .filter((name) => !["map", "entropy", "frequencies"].includes(name)); + controls.canTogglePanelLayout = false; + controls.panelLayout = "full"; + + return treeToo; +} \ No newline at end of file From 6094ba28ad3c2fc52e8a45590c864c26fa619f62 Mon Sep 17 00:00:00 2001 From: james hadfield Date: Tue, 11 Jun 2024 17:56:53 +1200 Subject: [PATCH 2/5] Allow filtering of data in second tree Trees define traitNames and traitValues via their `node_attrs` in the dataset JSON, for instance "traitName=country" and "traitValue=New Zealand". We populate the filtering dropdowns using these, but had previously only considered the main (LHS) tree. Behavioural changes: * traitName & traitValue present on both trees: no change. Filtering works as expected. * traitName on both trees, traitValue only on 2nd tree: functionality added here. * traitName only present on 2nd tree: this now works, but the app crashes from an uncaught error in the filter badges shown in the header. This will be fixed in the next commit. In addition, mutations and node ("sample") names unique to the 2nd tree are added to the available filters. Closes #1781 --- src/actions/recomputeReduxState.js | 4 +++ src/components/controls/filter.js | 46 ++++++++++++++++++++++++------ 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/src/actions/recomputeReduxState.js b/src/actions/recomputeReduxState.js index c8d641222..e802da9fa 100644 --- a/src/actions/recomputeReduxState.js +++ b/src/actions/recomputeReduxState.js @@ -1043,6 +1043,10 @@ function instantiateSecondTree(secondTreeDataset, metadata, genomeMap, secondTre treeToo.name = secondTreeName; updateMetadataStateViaSecondTree({...metadata}, secondTreeDataset, genomeMap) + const secondTreeColorings = convertColoringsListToDict(secondTreeDataset.meta?.colorings || []); + const stateCountAttrs = gatherTraitNames(treeToo.nodes, secondTreeColorings); + treeToo.totalStateCounts = countTraitsAcrossTree(treeToo.nodes, stateCountAttrs, false, true); + /* TODO: calc & display num tips in 2nd tree */ // metadata.secondTreeNumTips = calcTotalTipsInTree(treeToo.nodes); diff --git a/src/components/controls/filter.js b/src/components/controls/filter.js index 90159c39a..e379d7440 100644 --- a/src/components/controls/filter.js +++ b/src/components/controls/filter.js @@ -26,6 +26,8 @@ const DEBOUNCE_TIME = 200; totalStateCounts: state.tree.totalStateCounts, canFilterByGenotype: !!state.entropy.genomeMap, nodes: state.tree.nodes, + nodesSecondTree: state.treeToo?.nodes, + totalStateCountsSecondTree: state.treeToo?.totalStateCounts, measurementsFieldsMap: state.measurements.collectionToDisplay.fields, measurementsFiltersMap: state.measurements.collectionToDisplay.filters, measurementsFilters: state.controls.measurementsFilters @@ -66,16 +68,24 @@ class FilterData extends React.Component { * colorings). Within each trait, the values are alphabetical */ const coloringKeys = Object.keys(this.props.colorings||{}); - const unorderedTraitNames = Object.keys(this.props.totalStateCounts); + const unorderedTraitNames = [ + ...Object.keys(this.props.totalStateCounts), + ...Object.keys(this.props.totalStateCountsSecondTree), + ] const traitNames = [ ...coloringKeys.filter((name) => unorderedTraitNames.includes(name)), ...unorderedTraitNames.filter((name) => !coloringKeys.includes(name)) ] for (const traitName of traitNames) { - const traitData = this.props.totalStateCounts[traitName]; + const traitData = new Set([ + ...(this.props.totalStateCounts[traitName]?.keys() || []), + ...(this.props.totalStateCountsSecondTree?.[traitName]?.keys() || []), + ]); + + this.props.totalStateCounts[traitName]; const traitTitle = this.getFilterTitle(traitName); const filterValuesCurrentlyActive = new Set((this.props.activeFilters[traitName] || []).filter((x) => x.active).map((x) => x.value)); - for (const traitValue of Array.from(traitData.keys()).sort()) { + for (const traitValue of Array.from(traitData).sort()) { if (filterValuesCurrentlyActive.has(traitValue)) continue; options.push({ label: `${traitTitle} → ${traitValue}`, @@ -89,7 +99,12 @@ class FilterData extends React.Component { * mutations */ if (this.props.canFilterByGenotype) { - Array.from(collectGenotypeStates(this.props.nodes)) + const observedGenotypes = collectGenotypeStates(this.props.nodes); // set of "nuc:123A", "S:418K", etc + const observedGenotypesSecondTree = this.props.nodesSecondTree ? + collectGenotypeStates(this.props.nodesSecondTree).difference(observedGenotypes) : + new Set(); + Array.from(observedGenotypes) + .concat(Array.from(observedGenotypesSecondTree)) .sort() .forEach((o) => { options.push({ @@ -99,14 +114,29 @@ class FilterData extends React.Component { }); } - this.props.nodes + /** + * Add all (terminal) node names, calling each a "sample" + */ + const sampleNames = this.props.nodes .filter((n) => !n.hasChildren) - .forEach((n) => { + .map((n) => n.name); + sampleNames.forEach((name) => { options.push({ - label: `sample → ${n.name}`, - value: [strainSymbol, n.name] + label: `sample → ${name}`, + value: [strainSymbol, name] }); }); + if (this.props.nodesSecondTree) { + const seenNames = new Set(sampleNames); + this.props.nodesSecondTree + .filter((n) => !n.hasChildren && !seenNames.has(n.name)) + .forEach((n) => { + options.push({ + label: `sample → ${n.name}`, + value: [strainSymbol, n.name] + }); + }); + } if (this.props.measurementsOn && this.props.measurementsFiltersMap && this.props.measurementsFieldsMap) { this.props.measurementsFiltersMap.forEach(({values}, filterField) => { From 5d29e978eaf2dc98905199c97cd6eedcc0d7230b Mon Sep 17 00:00:00 2001 From: james hadfield Date: Wed, 12 Jun 2024 14:39:15 +1200 Subject: [PATCH 3/5] Update filter badges for multiple trees The previous implementation only reflected counts in the LHS tree, irregardless of any occurrences which may or may not be present in the RHS tree. We now communicate both. This also fixes a bug introduced in the previous commit where filtering to a traitName not present in the LHS tree would cause an uncaught exception. --- src/components/info/filtersSummary.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/components/info/filtersSummary.js b/src/components/info/filtersSummary.js index f905695ab..fce050231 100644 --- a/src/components/info/filtersSummary.js +++ b/src/components/info/filtersSummary.js @@ -33,6 +33,7 @@ const closeBracketSmall = a.value < b.value ? -1 : a.value > b.value ? 1 : 0) .map((item) => { let label = `${item.value}`; - if (filterName!==strainSymbol) label+= ` (${this.props.totalStateCounts[filterName].get(item.value)})`; + if (filterName!==strainSymbol) { + /* Add the _total_ occurrences in parentheses. We don't compute the intersections / visible values here - + e.g. we can have "England (5)" "North America (100)" even though such an intersection will deselect everything. + If we have two trees shown we show both values. + */ + const tree1count = this.props.totalStateCounts[filterName]?.get(item.value) ?? 0; + if (this.props.totalStateCountsSecondTree) { + const tree2count = this.props.totalStateCountsSecondTree[filterName]?.get(item.value) ?? 0; + label+=` (L: ${tree1count}, R: ${tree2count})`; + } else { + label+=` (${tree1count})`; + } + } return this.createIndividualBadge({filterName, item, label, onHoverMessage}); }); } From 7685fb27cfe16c2e1b38dbb2017d4966265382c6 Mon Sep 17 00:00:00 2001 From: james hadfield Date: Wed, 12 Jun 2024 14:50:58 +1200 Subject: [PATCH 4/5] Allow branch labels from second tree Branch labels were already rendered correctly on the RHS tree if the key was present in the LHS tree, so all that's required is to add label keys which are only observed in the RHS tree to appear in the sidebar branch label selector. Closes #1780 --- src/components/controls/choose-branch-labelling.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/components/controls/choose-branch-labelling.js b/src/components/controls/choose-branch-labelling.js index 20ed093fa..36539c4b2 100644 --- a/src/components/controls/choose-branch-labelling.js +++ b/src/components/controls/choose-branch-labelling.js @@ -11,7 +11,10 @@ import Toggle from "./toggle"; @connect((state) => ({ selected: state.controls.selectedBranchLabel, showAll: state.controls.showAllBranchLabels, - available: state.tree.availableBranchLabels, + available: Array.from( + (new Set(state.tree.availableBranchLabels)) + .union(new Set(state.treeToo?.availableBranchLabels ?? [])) + ), canRenderBranchLabels: state.controls.canRenderBranchLabels })) class ChooseBranchLabelling extends React.Component { From 99e5336eb8ebb2e9ec0df75b120f3a976e586b53 Mon Sep 17 00:00:00 2001 From: james hadfield Date: Wed, 12 Jun 2024 15:35:00 +1200 Subject: [PATCH 5/5] changelog --- CHANGELOG.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5473fc17d..ae6dd20d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,12 @@ # Changelog -* remove missing warning then dataset JSON has no `.meta.geo_resolutions` field ([#1791](https://github.com/nextstrain/auspice/pull/1791)) +* A number of improvements when viewing multiple trees (tangletrees) ([#1788](https://github.com/nextstrain/auspice/pull/1788)) + * Attributes only present on the RHS tree are now available as filter options, as well as genotypes and node names unique to the RHS tree. + * Filter badges (shown in the header) now indicate how many matches are present in both trees (formerly only the LHS tree was considered) + * Branch labels unique to the RHS tree are now available +* remove missing warning when dataset JSON has no `.meta.geo_resolutions` field ([#1791](https://github.com/nextstrain/auspice/pull/1791)) * Add support for Node.js version 22. ([#1779](https://github.com/nextstrain/auspice/pull/1779)) - ## version 2.54.3 - 2024/06/12