From 09407cf9b8b33e2095122247f3352eeb2298ae9e Mon Sep 17 00:00:00 2001 From: James Hadfield Date: Tue, 18 May 2021 16:47:34 +1200 Subject: [PATCH 1/5] Allow non-continuous scatterplot variables This implements a requested improvement to the original scatterplot implementation. The implementation hinges on two changes: (1) The collection of values for a given variable (e.g. x-var) need to be computed and passed to PhyloTree to act as the scale's domain. We reuse the colorScale machinery here, which could be optimised (see todo messages in code), but this has the advantage that the domain ordering matches the legend (unless user supplied). (2) PhyloTree needed to be modified to use non-linear scales, in this case `pointScale`. This commit should be fully functional, however there are some future improvements to be made: (i) Grid text is obscured and unreadable when there are many entries in the domain. (ii) Genotypes and Boolean scales are not yet available. (iii) Jitter should be added to nodes to avoid obfuscation. --- src/actions/recomputeReduxState.js | 2 +- src/components/controls/choose-layout.js | 50 +++++---- src/components/tree/phyloTree/grid.js | 11 ++ src/components/tree/phyloTree/layouts.js | 112 +++++++++++++-------- src/components/tree/phyloTree/phyloTree.js | 3 - src/util/colorScale.js | 8 +- src/util/scatterplotHelpers.js | 48 ++++++++- 7 files changed, 166 insertions(+), 68 deletions(-) diff --git a/src/actions/recomputeReduxState.js b/src/actions/recomputeReduxState.js index 734646a1e..b6e4c3058 100644 --- a/src/actions/recomputeReduxState.js +++ b/src/actions/recomputeReduxState.js @@ -568,7 +568,7 @@ const checkAndCorrectErrorsInState = (state, metadata, query, tree, viewingNarra // todo: these should be JSON definable (via display_defaults) if (state.layout==="scatter" || state.layout==="clock") { state.scatterVariables = validateScatterVariables( - state.scatterVariables, metadata.colorings, state.distanceMeasure, state.colorBy, state.layout==="clock" + state, metadata, tree, state.layout==="clock" ); if (query.scatterX && query.scatterX!==state.scatterVariables.x) delete query.scatterX; if (query.scatterY && query.scatterY!==state.scatterVariables.y) delete query.scatterY; diff --git a/src/components/controls/choose-layout.js b/src/components/controls/choose-layout.js index d914242f8..7c4dc0a23 100644 --- a/src/components/controls/choose-layout.js +++ b/src/components/controls/choose-layout.js @@ -9,7 +9,7 @@ import { withTranslation } from 'react-i18next'; import Select from "react-select/lib/Select"; import * as icons from "../framework/svg-icons"; import { controlsWidth } from "../../util/globals"; -import { collectAvailableScatterVariables, validateScatterVariables} from "../../util/scatterplotHelpers"; +import { collectAvailableScatterVariables, validateScatterVariables, addScatterAxisInfo} from "../../util/scatterplotHelpers"; import { CHANGE_LAYOUT } from "../../actions/types"; import { SidebarSubtitle, SidebarButton } from "./styles"; import Toggle from "./toggle"; @@ -30,8 +30,9 @@ export const RowContainer = styled.div` layout: state.controls.layout, scatterVariables: state.controls.scatterVariables, colorBy: state.controls.colorBy, - distanceMeasure: state.controls.distanceMeasure, - colorings: state.metadata.colorings, + controls: state.controls, + tree: state.tree, + metadata: state.metadata, showTreeToo: state.controls.showTreeToo, branchLengthsToDisplay: state.controls.branchLengthsToDisplay }; @@ -48,12 +49,15 @@ class ChooseLayout extends React.Component { const scatterVariables = modifiedScatterVariables ? {...this.props.scatterVariables, ...modifiedScatterVariables} : this.props.scatterVariables; + if (layout==="scatter" && (!scatterVariables.xContinuous || !scatterVariables.yContinuous)) { + scatterVariables.showRegression= false; + } this.props.dispatch({type: CHANGE_LAYOUT, layout, scatterVariables}); }; } renderScatterplotAxesSelector() { - const options = collectAvailableScatterVariables(this.props.colorings); + const options = collectAvailableScatterVariables(this.props.metadata.colorings); const selectedX = options.filter((o) => o.value===this.props.scatterVariables.x)[0]; const selectedY = options.filter((o) => o.value===this.props.scatterVariables.y)[0]; const miscSelectProps = {options, clearable: false, searchable: false, multi: false, valueKey: "label"}; @@ -66,7 +70,10 @@ class ChooseLayout extends React.Component { this.updateLayout("scatter", {y: value.value, yLabel: value.label})} + onChange={(value) => this.updateLayout( + "scatter", + addScatterAxisInfo({y: value.value, yLabel: value.label}, "y", this.props.controls, this.props.tree, this.props.metadata) + )} /> @@ -98,15 +108,21 @@ class ChooseLayout extends React.Component { />
- - this.updateLayout(this.props.layout, {showRegression: !this.props.scatterVariables.showRegression})} - label={"Show regression"} - /> - -
+ { + (this.props.scatterVariables.xContinuous && this.props.scatterVariables.yContinuous) && ( + <> + + this.updateLayout(this.props.layout, {showRegression: !this.props.scatterVariables.showRegression})} + label={"Show regression"} + /> + +
+ + ) + } ); } @@ -157,7 +173,7 @@ class ChooseLayout extends React.Component { selected={selected === "clock"} onClick={() => this.updateLayout( "clock", - validateScatterVariables(this.props.scatterVariables, this.props.colorings, this.props.distanceMeasure, this.props.colorBy, true) + validateScatterVariables(this.props.controls, this.props.metadata, this.props.tree, true) )} > {t("sidebar:clock")} @@ -174,7 +190,7 @@ class ChooseLayout extends React.Component { selected={selected === "scatter"} onClick={() => this.updateLayout( "scatter", - validateScatterVariables(this.props.scatterVariables, this.props.colorings, this.props.distanceMeasure, this.props.colorBy, false) + validateScatterVariables(this.props.controls, this.props.metadata, this.props.tree, false) )} > {t("sidebar:scatter")} diff --git a/src/components/tree/phyloTree/grid.js b/src/components/tree/phyloTree/grid.js index 3952a56dd..3b0a3d0f3 100644 --- a/src/components/tree/phyloTree/grid.js +++ b/src/components/tree/phyloTree/grid.js @@ -245,6 +245,13 @@ export const addGrid = function addGrid() { (this.layout!=="scatter" && this.distance==="num_date") ) { xGridPoints = computeTemporalGridPoints(xmin, xmax, xAxisPixels, "x"); + } else if (this.layout==="scatter" && !this.scatterVariables.xContinuous) { + xGridPoints = { + majorGridPoints: this.xScale.domain().map((name) => ({ + name, visibility: "visible", axis: "x", position: name + })), + minorGridPoints: [] + }; } else { xGridPoints = computeNumericGridPoints(xmin, xmax, layout, this.params.minorTicks, "x"); } @@ -319,6 +326,10 @@ export const addGrid = function addGrid() { const yAxisPixels = this.yScale.range()[1] - this.yScale.range()[0]; const temporalGrid = computeTemporalGridPoints(ymin, ymax, yAxisPixels, "y"); majorGridPoints.push(...temporalGrid.majorGridPoints); + } else if (this.layout==="scatter" && !this.scatterVariables.yContinuous) { + majorGridPoints.push(...this.yScale.domain().map((name) => ({ + name, visibility: "visible", axis: "y", position: name + }))); } else { const numericGrid = computeNumericGridPoints(ymin, ymax, layout, 1, "y"); majorGridPoints.push(...numericGrid.majorGridPoints); diff --git a/src/components/tree/phyloTree/layouts.js b/src/components/tree/phyloTree/layouts.js index 034a0a6bb..1127f5e2b 100644 --- a/src/components/tree/phyloTree/layouts.js +++ b/src/components/tree/phyloTree/layouts.js @@ -1,6 +1,8 @@ /* eslint-disable no-multi-spaces */ /* eslint-disable space-infix-ops */ import { min, max } from "d3-array"; +import scaleLinear from "d3-scale/src/linear"; +import {point as scalePoint} from "d3-scale/src/band"; import { addLeafCount} from "./helpers"; import { calculateRegressionThroughRoot, calculateRegressionWithFreeIntercept } from "./regression"; import { timerStart, timerEnd } from "../../../util/perf"; @@ -259,10 +261,23 @@ export const setDistance = function setDistance(distanceAttribute) { /** - * sets the range of the scales used to map the x,y coordinates to the screen + * Initializes and sets the range of the scales (this.xScale, this.yScale) + * which are used to map the x,y coordinates to the screen * @param {margins} -- object with "right, left, top, bottom" margins */ export const setScales = function setScales(margins) { + + if (this.layout==="scatter" && !this.scatterVariables.xContinuous) { + this.xScale = scalePoint().round(true).align(0.5); + } else { + this.xScale = scaleLinear(); + } + if (this.layout==="scatter" && !this.scatterVariables.yContinuous) { + this.yScale = scalePoint().round(true).align(0.5); + } else { + this.yScale = scaleLinear(); + } + const width = parseInt(this.svg.attr("width"), 10); const height = parseInt(this.svg.attr("height"), 10); if (this.layout === "radial" || this.layout === "unrooted") { @@ -276,7 +291,7 @@ export const setScales = function setScales(margins) { this.yScale.range([0.5 * ySlack + margins["top"] || 0, height - 0.5 * ySlack - (margins["bottom"] || 0)]); } else { - // for rectancular layout, allow flipping orientation of left right and top/botton + // for rectangular layout, allow flipping orientation of left/right and top/bottom if (this.params.orientation[0] > 0) { this.xScale.range([margins["left"] || 0, width - (margins["right"] || 0)]); } else { @@ -331,62 +346,75 @@ export const mapToScreen = function mapToScreen() { /* set the range of the x & y scales */ this.setScales(tmpMargins); - /* find minimum & maximum x & y values */ - let [minY, maxY, minX, maxX] = [1000000, -100000, 1000000, -100000]; let nodesInDomain = this.nodes.filter((d) => d.inView && d.y!==undefined && d.x!==undefined); // 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.forEach((d) => { - if (d.x > maxX) maxX = d.x; - if (d.y > maxY) maxY = d.y; - if (d.x < minX) minX = d.x; - if (d.y < minY) minY = d.y; - }); - /* fixes state of 0 length domain */ - if (minX === maxX) { - minX -= 0.005; - maxX += 0.005; - } - - /* slightly pad min and max y to account for small clades */ - if (inViewTerminalNodes.length < 30) { - const delta = 0.05 * (maxY - minY); - minY -= delta; - maxY += delta; + /* Compute the domains to pass to the d3 scales for the x & y axes */ + let xDomain, yDomain, spanX, spanY; + if (this.layout!=="scatter" || this.scatterVariables.xContinuous) { + let [minX, maxX] = [1000000, -100000]; + nodesInDomain.forEach((d) => { + if (d.x < minX) minX = d.x; + if (d.x > maxX) maxX = d.x; + }); + /* fixes state of 0 length domain */ + if (minX === maxX) { + minX -= 0.005; + maxX += 0.005; + } + /* Don't allow tiny x-axis domains -- e.g. if zoomed into a polytomy where the + divergence values are all tiny, then we don't want to display the tree topology */ + const minimumXAxisSpan = 1E-8; + spanX = maxX-minX; + if (spanX < minimumXAxisSpan) { + maxX = minimumXAxisSpan - minX; + spanX = minimumXAxisSpan; + } + xDomain = [minX, maxX]; + } else { + const seenValues = new Set(nodesInDomain.map((d) => d.x)); + xDomain = this.scatterVariables.xDomain.filter((v) => seenValues.has(v)); } - /* Don't allow tiny x-axis domains -- e.g. if zoomed into a polytomy where the - divergence values are all tiny, then we don't want to display the tree topology */ - const minimumXAxisSpan = 1E-8; - let spanX = maxX-minX; - if (spanX < minimumXAxisSpan) { - maxX = minimumXAxisSpan - minX; - spanX = minimumXAxisSpan; + if (this.layout!=="scatter" || this.scatterVariables.yContinuous) { + let [minY, maxY] = [1000000, -100000]; + nodesInDomain.forEach((d) => { + if (d.y < minY) minY = d.y; + if (d.y > maxY) maxY = d.y; + }); + /* slightly pad min and max y to account for small clades */ + if (inViewTerminalNodes.length < 30) { + const delta = 0.05 * (maxY - minY); + minY -= delta; + maxY += delta; + } + spanY = maxY-minY; + yDomain = [minY, maxY]; + } else { + const seenValues = new Set(nodesInDomain.map((d) => d.y)); + yDomain = this.scatterVariables.yDomain.filter((v) => seenValues.has(v)); } - /* set the domain of the x & y scales */ + /* Radial / Unrooted layouts need to be square since branch lengths + depend on this */ if (this.layout === "radial" || this.layout === "unrooted") { - // handle "radial and unrooted differently since they need to be square - // since branch length move in x and y direction - // TODO: should be tied to svg dimensions - const spanY = maxY-minY; const maxSpan = max([spanY, spanX]); const ySlack = (spanX>spanY) ? (spanX-spanY)*0.5 : 0.0; const xSlack = (spanX {this.strainToNode[phylonode.n.name] = phylonode;}); diff --git a/src/util/colorScale.js b/src/util/colorScale.js index 193898aa3..8cbc0ec73 100644 --- a/src/util/colorScale.js +++ b/src/util/colorScale.js @@ -33,7 +33,7 @@ export const calcColorScale = (colorBy, controls, tree, treeToo, metadata) => { const colorings = metadata.colorings; const treeTooNodes = treeToo ? treeToo.nodes : undefined; let continuous = false; - let colorScale, legendValues, legendBounds, legendLabels; + let colorScale, legendValues, legendBounds, legendLabels, domain; let genotype; if (isColorByGenotype(colorBy) && controls.geneLength) { @@ -62,6 +62,10 @@ export const calcColorScale = (colorBy, controls, tree, treeToo, metadata) => { throw new Error(`ColorBy ${colorBy} invalid type -- ${scaleType}`); } + /* We store a copy of the `domain`, which for non-continuous scales is a ordered list of values for this colorBy, + for future list */ + if (scaleType !== 'continuous') domain = legendValues.slice(); + /* Use user-defined `legend` data (if any) to define custom legend elements */ const legendData = parseUserProvidedLegendData(colorings[colorBy].legend, legendValues, scaleType); if (legendData) { @@ -92,6 +96,7 @@ export const calcColorScale = (colorBy, controls, tree, treeToo, metadata) => { legendBounds, legendLabels, genotype, + domain, scaleType: scaleType, visibleLegendValues: visibleLegendValues }; @@ -107,6 +112,7 @@ export const calcColorScale = (colorBy, controls, tree, treeToo, metadata) => { legendBounds: createLegendBounds(["unknown"]), genotype: null, scaleType: null, + domain: null, visibleLegendValues: ["unknown"] }; } diff --git a/src/util/scatterplotHelpers.js b/src/util/scatterplotHelpers.js index 84f44b111..d4b10b766 100644 --- a/src/util/scatterplotHelpers.js +++ b/src/util/scatterplotHelpers.js @@ -1,10 +1,12 @@ - +import { calcColorScale } from "./colorScale"; export function collectAvailableScatterVariables(colorings) { // todo: genotype (special case) + // Note for implementation of genotype - be careful with calls to `calcColorScale` (e.g. from `validateScatterVariables`) + // as one of the side effects of that function is setting the genotype on nodes const options = Object.keys(colorings) .filter((key) => key!=="gt") - .filter((key) => colorings[key].type==="continuous") // work needed to render non-continuous scales in PhyloTree + .filter((key) => colorings[key].type!=="boolean") .map((key) => ({ value: key, label: colorings[key].title || key @@ -16,8 +18,10 @@ export function collectAvailableScatterVariables(colorings) { /** * Return a (validated) `scatterVariables` object, given any existing scatterVariables (which may themselves be invalid) */ -export function validateScatterVariables(existingScatterVariables={}, colorings, distanceMeasure, colorBy, isClock) { - const availableOptions = collectAvailableScatterVariables(colorings); +export function validateScatterVariables(controls, metadata, tree, isClock) { + const {distanceMeasure, colorBy } = controls; + const existingScatterVariables = {...controls.scatterVariables}; + const availableOptions = collectAvailableScatterVariables(metadata.colorings); const scatterVariables = {}; // default is to show branches, unless the existing state says otherwise scatterVariables.showBranches = Object.prototype.hasOwnProperty.call(existingScatterVariables, "showBranches") ? @@ -35,6 +39,10 @@ export function validateScatterVariables(existingScatterVariables={}, colorings, const yOption = _getFirstMatchingOption(availableOptions, [existingScatterVariables.y, colorBy], xOption.value); scatterVariables.y = yOption.value; scatterVariables.yLabel = yOption.label; + for (const axis of ["x", "y"]) { + addScatterAxisInfo(scatterVariables, axis, controls, tree, metadata); + } + if (!scatterVariables.xContinuous || !scatterVariables.yContinuous) scatterVariables.showRegression= false; } return scatterVariables; } @@ -62,3 +70,35 @@ function _getFirstMatchingOption(options, tryTheseFirst, notThisValue) { } return undefined; } + +/** + * Given a colorBy and a scatterplot axes, calculate whether the axes is + * continous or not. If not, calculate the domain. For this we use the same code + * as colour scale construction to find a domain. There are opportunities for refactoring here + * to improve performance, as we only use two properties from the returned object. + * Similarly, if one of the axes variables is the current colorBy, we could avoid recalculating this. + * @param {Object} scatterVariables + * @param {string} axis values: "x" or "y" + * @param {Object} controls + * @param {Object} controls + * @param {Object} metadata + * @returns {Object} the `scatterVariables` param with modifications + * @sideEffects adds keys `${axis}Continuous`, `${axis}Domain` to the `scatterVariables` param. + */ +export function addScatterAxisInfo(scatterVariables, axis, controls, tree, metadata) { + const axisVar = scatterVariables[axis]; + if (["div", "num_date"].includes(axisVar) || metadata.colorings[axisVar].type==="continuous") { + scatterVariables[`${axis}Continuous`] = true; + scatterVariables[`${axis}Domain`] = undefined; + return scatterVariables; + } + const {domain, scaleType} = calcColorScale(scatterVariables[axis], controls, tree, null, metadata); + if (scaleType==="continuous") { + scatterVariables[`${axis}Continuous`] = true; + scatterVariables[`${axis}Domain`] = undefined; + return scatterVariables; + } + scatterVariables[`${axis}Continuous`] = false; + scatterVariables[`${axis}Domain`] = domain.slice(); + return scatterVariables; +} From e40b03f95d556a966cf79176c8c9efcec965c2ac Mon Sep 17 00:00:00 2001 From: James Hadfield Date: Fri, 21 May 2021 16:58:09 +1200 Subject: [PATCH 2/5] Layout changes occur via redux thunk This commit is in preparation for allowing genotype to be a scatterplot variable. This will complicate the allowable scatterplot variables and force these to update upon colorBy changes. This is much cleaner if layout is changed in a thunk. --- src/actions/layout.js | 37 +++++++++++++++ src/components/controls/choose-layout.js | 58 ++++++------------------ src/reducers/controls.js | 2 +- 3 files changed, 51 insertions(+), 46 deletions(-) create mode 100644 src/actions/layout.js diff --git a/src/actions/layout.js b/src/actions/layout.js new file mode 100644 index 000000000..098129a0f --- /dev/null +++ b/src/actions/layout.js @@ -0,0 +1,37 @@ +import { CHANGE_LAYOUT } from "./types"; +import { validateScatterVariables, addScatterAxisInfo} from "../util/scatterplotHelpers"; + +/** + * Redux Thunk to change a layout, including aspects of the scatterplot / clock layouts. + */ +export const changeLayout = ({layout, showBranches, showRegression, x, xLabel, y, yLabel}) => { + return (dispatch, getState) => { + if (window.NEXTSTRAIN && window.NEXTSTRAIN.animationTickReference) return; + const { controls, tree, metadata } = getState(); + + if (layout==="rect" || layout==="unrooted" || layout==="radial") { + dispatch({type: CHANGE_LAYOUT, layout, scatterVariables: controls.scatterVariables, canRenderBranchLabels: true}); + return; + } + + let scatterVariables = (layout==="clock" || layout==="scatter") ? + validateScatterVariables(controls, metadata, tree, layout==="clock") : // occurs when switching to this layout + controls.scatterVariables; + + if (x && xLabel) scatterVariables = {...scatterVariables, ...addScatterAxisInfo({x, xLabel}, "x", controls, tree, metadata)}; + if (y && yLabel) scatterVariables = {...scatterVariables, ...addScatterAxisInfo({y, yLabel}, "y", controls, tree, metadata)}; + if (showBranches!==undefined) scatterVariables.showBranches = showBranches; + if (showRegression!==undefined) scatterVariables.showRegression = showRegression; + if (layout==="scatter" && (!scatterVariables.xContinuous || !scatterVariables.yContinuous)) { + scatterVariables.showRegression= false; + } + + dispatch({ + type: CHANGE_LAYOUT, + layout: layout || controls.layout, + scatterVariables: {...scatterVariables}, // ensures redux is aware of change + canRenderBranchLabels: scatterVariables.showBranches + }); + + }; +}; diff --git a/src/components/controls/choose-layout.js b/src/components/controls/choose-layout.js index 7c4dc0a23..44fb000ae 100644 --- a/src/components/controls/choose-layout.js +++ b/src/components/controls/choose-layout.js @@ -1,6 +1,3 @@ -/* eslint-disable react/jsx-no-bind */ -/* ^^^ We can get away with this because doesn't rerender frequently, but fixes are welcome */ - import React from "react"; import PropTypes from 'prop-types'; import { connect } from "react-redux"; @@ -9,9 +6,9 @@ import { withTranslation } from 'react-i18next'; import Select from "react-select/lib/Select"; import * as icons from "../framework/svg-icons"; import { controlsWidth } from "../../util/globals"; -import { collectAvailableScatterVariables, validateScatterVariables, addScatterAxisInfo} from "../../util/scatterplotHelpers"; -import { CHANGE_LAYOUT } from "../../actions/types"; +import { collectAvailableScatterVariables} from "../../util/scatterplotHelpers"; import { SidebarSubtitle, SidebarButton } from "./styles"; +import { changeLayout } from "../../actions/layout"; import Toggle from "./toggle"; @@ -29,10 +26,7 @@ export const RowContainer = styled.div` return { layout: state.controls.layout, scatterVariables: state.controls.scatterVariables, - colorBy: state.controls.colorBy, - controls: state.controls, - tree: state.tree, - metadata: state.metadata, + colorings: state.metadata.colorings, showTreeToo: state.controls.showTreeToo, branchLengthsToDisplay: state.controls.branchLengthsToDisplay }; @@ -42,22 +36,8 @@ class ChooseLayout extends React.Component { layout: PropTypes.string.isRequired, dispatch: PropTypes.func.isRequired } - constructor(props) { - super(props); - this.updateLayout = (layout, modifiedScatterVariables=undefined) => { - if (window.NEXTSTRAIN && window.NEXTSTRAIN.animationTickReference) return; - const scatterVariables = modifiedScatterVariables ? - {...this.props.scatterVariables, ...modifiedScatterVariables} : - this.props.scatterVariables; - if (layout==="scatter" && (!scatterVariables.xContinuous || !scatterVariables.yContinuous)) { - scatterVariables.showRegression= false; - } - this.props.dispatch({type: CHANGE_LAYOUT, layout, scatterVariables}); - }; - } - renderScatterplotAxesSelector() { - const options = collectAvailableScatterVariables(this.props.metadata.colorings); + const options = collectAvailableScatterVariables(this.props.colorings); const selectedX = options.filter((o) => o.value===this.props.scatterVariables.x)[0]; const selectedY = options.filter((o) => o.value===this.props.scatterVariables.y)[0]; const miscSelectProps = {options, clearable: false, searchable: false, multi: false, valueKey: "label"}; @@ -70,10 +50,7 @@ class ChooseLayout extends React.Component { this.updateLayout( - "scatter", - addScatterAxisInfo({y: value.value, yLabel: value.label}, "y", this.props.controls, this.props.tree, this.props.metadata) - )} + onChange={(value) => this.props.dispatch(changeLayout({y: value.value, yLabel: value.label}))} /> @@ -103,7 +77,7 @@ class ChooseLayout extends React.Component { this.updateLayout(this.props.layout, {showBranches: !this.props.scatterVariables.showBranches})} + callback={() => this.props.dispatch(changeLayout({showBranches: !this.props.scatterVariables.showBranches}))} label={"Show branches"} /> @@ -115,7 +89,7 @@ class ChooseLayout extends React.Component { this.updateLayout(this.props.layout, {showRegression: !this.props.scatterVariables.showRegression})} + callback={() => this.props.dispatch(changeLayout({showRegression: !this.props.scatterVariables.showRegression}))} label={"Show regression"} /> @@ -140,7 +114,7 @@ class ChooseLayout extends React.Component { this.updateLayout("rect")} + onClick={() => this.props.dispatch(changeLayout({layout: "rect"}))} > {t("sidebar:rectangular")} @@ -149,7 +123,7 @@ class ChooseLayout extends React.Component { this.updateLayout("radial")} + onClick={() => this.props.dispatch(changeLayout({layout: "radial"}))} > {t("sidebar:radial")} @@ -158,7 +132,7 @@ class ChooseLayout extends React.Component { this.updateLayout("unrooted")} + onClick={() => this.props.dispatch(changeLayout({layout: "unrooted"}))} > {t("sidebar:unrooted")} @@ -171,10 +145,7 @@ class ChooseLayout extends React.Component { this.updateLayout( - "clock", - validateScatterVariables(this.props.controls, this.props.metadata, this.props.tree, true) - )} + onClick={() => this.props.dispatch(changeLayout({layout: "clock"}))} > {t("sidebar:clock")} @@ -188,10 +159,7 @@ class ChooseLayout extends React.Component { this.updateLayout( - "scatter", - validateScatterVariables(this.props.controls, this.props.metadata, this.props.tree, false) - )} + onClick={() => this.props.dispatch(changeLayout({layout: "scatter"}))} > {t("sidebar:scatter")} diff --git a/src/reducers/controls.js b/src/reducers/controls.js index 39171b7ac..80c13efb4 100644 --- a/src/reducers/controls.js +++ b/src/reducers/controls.js @@ -123,7 +123,7 @@ const Controls = (state = getDefaultControlsState(), action) => { case types.CHANGE_LAYOUT: return Object.assign({}, state, { layout: action.layout, - canRenderBranchLabels: (action.layout!=="scatter" && action.layout!=="clock") || (action.scatterVariables && action.scatterVariables.showBranches), + canRenderBranchLabels: action.canRenderBranchLabels, scatterVariables: action.scatterVariables, /* temporal confidence can only be displayed for rectangular trees */ temporalConfidence: Object.assign({}, state.temporalConfidence, { From d55a888320cb2a4b6d2fa896ccb8936832589a59 Mon Sep 17 00:00:00 2001 From: James Hadfield Date: Fri, 21 May 2021 17:29:34 +1200 Subject: [PATCH 3/5] Allow genotype to be scatterplot variable Genotype is treated differently to other colorings in two important ways: (1) it can change value, for instance when changing the colorby to another genotype position and (2) it is stored in a different place to other colorings. These require scatterplot logic to be more complex as actions are no longer separate - we now require a NEW_COLOURS action to potentially update the layout which was formerly within the remit of the CHANGE_LAYOUT actions. This is achieved through a middleware layer. This implementation makes it clear that jitter and better domain spacing are crucial for scatterplots. --- src/components/controls/choose-layout.js | 3 +- src/components/tree/phyloTree/layouts.js | 6 +++ src/middleware/scatterplot.js | 64 ++++++++++++++++++++++++ src/reducers/controls.js | 3 ++ src/store/index.js | 2 + src/util/colorScale.js | 3 +- src/util/getGenotype.js | 5 ++ src/util/scatterplotHelpers.js | 44 ++++++++++------ 8 files changed, 113 insertions(+), 17 deletions(-) create mode 100644 src/middleware/scatterplot.js diff --git a/src/components/controls/choose-layout.js b/src/components/controls/choose-layout.js index 44fb000ae..e0b75e5cd 100644 --- a/src/components/controls/choose-layout.js +++ b/src/components/controls/choose-layout.js @@ -27,6 +27,7 @@ export const RowContainer = styled.div` layout: state.controls.layout, scatterVariables: state.controls.scatterVariables, colorings: state.metadata.colorings, + colorBy: state.controls.colorBy, showTreeToo: state.controls.showTreeToo, branchLengthsToDisplay: state.controls.branchLengthsToDisplay }; @@ -37,7 +38,7 @@ class ChooseLayout extends React.Component { dispatch: PropTypes.func.isRequired } renderScatterplotAxesSelector() { - const options = collectAvailableScatterVariables(this.props.colorings); + const options = collectAvailableScatterVariables(this.props.colorings, this.props.colorBy); const selectedX = options.filter((o) => o.value===this.props.scatterVariables.x)[0]; const selectedY = options.filter((o) => o.value===this.props.scatterVariables.y)[0]; const miscSelectProps = {options, clearable: false, searchable: false, multi: false, valueKey: "label"}; diff --git a/src/components/tree/phyloTree/layouts.js b/src/components/tree/phyloTree/layouts.js index 1127f5e2b..2de7f6ff7 100644 --- a/src/components/tree/phyloTree/layouts.js +++ b/src/components/tree/phyloTree/layouts.js @@ -86,6 +86,9 @@ export const scatterplotLayout = function scatterplotLayout() { if (this.scatterVariables.x==="div") { d.x = getDivFromNode(d.n); d.px = getDivFromNode(d.n.parent); + } else if (this.scatterVariables.x==="gt") { + d.x = d.n.currentGt; + d.px = d.n.parent.currentGt; } else { d.x = getTraitFromNode(d.n, this.scatterVariables.x); d.px = getTraitFromNode(d.n.parent, this.scatterVariables.x); @@ -94,6 +97,9 @@ export const scatterplotLayout = function scatterplotLayout() { if (this.scatterVariables.y==="div") { d.y = getDivFromNode(d.n); d.py = getDivFromNode(d.n.parent); + } else if (this.scatterVariables.y==="gt") { + d.y = d.n.currentGt; + d.py = d.n.parent.currentGt; } else { d.y = getTraitFromNode(d.n, this.scatterVariables.y); d.py = getTraitFromNode(d.n.parent, this.scatterVariables.y); diff --git a/src/middleware/scatterplot.js b/src/middleware/scatterplot.js new file mode 100644 index 000000000..348c65e28 --- /dev/null +++ b/src/middleware/scatterplot.js @@ -0,0 +1,64 @@ +/** + * This middleware adds scatterplot-related information to the NEW_COLORS action + * when necessary. By overloading a single action, rather than dispatching twice, + * we guarantee that only one call to PhyloTree's `change()` function is made + * which avoids hard-to-debug rendering problems where d3 is out-of-sync with state + */ + +import { NEW_COLORS } from "../actions/types"; +import { isColorByGenotype, makeGenotypeLabel } from "../util/getGenotype"; +import { addScatterAxisGivenColorScale, getFirstMatchingScatterVariable, collectAvailableScatterVariables, addScatterAxisInfo } from "../util/scatterplotHelpers"; + +export const keepScatterplotStateInSync = (store) => (next) => (action) => { + if (action.type!==NEW_COLORS) return next(action); + + const { controls, metadata, tree } = store.getState(); // state before reaching reducers + if (controls.layout!=="scatter" || !isColorByGenotype(controls.colorBy)) { + return next(action); + } + + const scatterVariables = {...controls.scatterVariables}; + let changed = false; + + /** we consider two cases where previous colorBy was genotype. Firstly, + * if the genotype has changed, and the scatterplot was rendering the genotype + * then we need to update the scatterplot variable (incl domain) to the new genotype. + * (Without this the colours update appropriately, but not the node positions) + */ + const genotypeLabel = makeGenotypeLabel(action.colorBy); // false if new colorBy is not a genotype + if (genotypeLabel) { + changed = true; + if (scatterVariables.x==="gt") { + scatterVariables.xLabel = genotypeLabel; + addScatterAxisGivenColorScale(scatterVariables, action.colorScale, "x"); + } + if (scatterVariables.y==="gt") { + scatterVariables.yLabel = genotypeLabel; + addScatterAxisGivenColorScale(scatterVariables, action.colorScale, "y"); + } + } + + /** The second case occurs when the action is changing the colorBy from a genotype + * to a non genotype, and a (or both) scatterplot axis was rendering genotype. + * Here we need to move to a different scatterplot variable, and we default to the + * new colorBy. + */ + if (!genotypeLabel && (scatterVariables.x==="gt" || scatterVariables.y==="gt")) { + changed=true; + const availableOptions = collectAvailableScatterVariables(metadata.colorings, action.colorBy); + if (scatterVariables.y==="gt") { + const {value, label} = getFirstMatchingScatterVariable(availableOptions, [action.colorBy], scatterVariables.x); + scatterVariables.y = value; + scatterVariables.yLabel = label; + addScatterAxisInfo(scatterVariables, "y", controls, tree, metadata); + } + if (scatterVariables.x==="gt") { + const {value, label} = getFirstMatchingScatterVariable(availableOptions, [action.colorBy], scatterVariables.y); + scatterVariables.x = value; + scatterVariables.xLabel = label; + addScatterAxisInfo(scatterVariables, "x", controls, tree, metadata); + } + } + if (changed) return next({...action, scatterVariables}); + return next(action); +}; diff --git a/src/reducers/controls.js b/src/reducers/controls.js index 80c13efb4..a3cc7525d 100644 --- a/src/reducers/controls.js +++ b/src/reducers/controls.js @@ -219,6 +219,9 @@ const Controls = (state = getDefaultControlsState(), action) => { colorScale: action.colorScale, colorByConfidence: doesColorByHaveConfidence(state, action.colorBy) }); + if (action.scatterVariables) { + newState.scatterVariables = action.scatterVariables; + } return newState; } case types.CHANGE_GEO_RESOLUTION: diff --git a/src/store/index.js b/src/store/index.js index a6c34d6a6..31560da01 100644 --- a/src/store/index.js +++ b/src/store/index.js @@ -3,10 +3,12 @@ import thunk from "redux-thunk"; import { changeURLMiddleware } from "../middleware/changeURL"; import rootReducer from "../reducers"; import { loggingMiddleware } from "../middleware/logActions"; // eslint-disable-line no-unused-vars +import { keepScatterplotStateInSync } from "../middleware/scatterplot"; const configureStore = (initialState) => { const middleware = [ thunk, + keepScatterplotStateInSync, changeURLMiddleware, // eslint-disable-line comma-dangle // loggingMiddleware ]; diff --git a/src/util/colorScale.js b/src/util/colorScale.js index 8cbc0ec73..9ebff7ee4 100644 --- a/src/util/colorScale.js +++ b/src/util/colorScale.js @@ -43,6 +43,7 @@ export const calcColorScale = (colorBy, controls, tree, treeToo, metadata) => { const scaleType = genotype ? "categorical" : colorings[colorBy].type; if (genotype) { ({legendValues, colorScale} = createScaleForGenotype(tree.nodes, controls.mutType)); + domain = [...legendValues]; } else if (colorings && colorings[colorBy]) { if (scaleType === "continuous") { ({continuous, colorScale, legendBounds, legendValues} = @@ -112,7 +113,7 @@ export const calcColorScale = (colorBy, controls, tree, treeToo, metadata) => { legendBounds: createLegendBounds(["unknown"]), genotype: null, scaleType: null, - domain: null, + domain: [], visibleLegendValues: ["unknown"] }; } diff --git a/src/util/getGenotype.js b/src/util/getGenotype.js index 1c1fbecc3..99dcb144d 100644 --- a/src/util/getGenotype.js +++ b/src/util/getGenotype.js @@ -104,3 +104,8 @@ export const decodeGenotypeFilters = (query) => { .map((value) => ({active: true, value})); // all URL filters _start_ active }; +export const makeGenotypeLabel = (colorBy) => { + const genotype = isColorByGenotype(colorBy) ? decodeColorByGenotype(colorBy) : false; + if (!genotype) return false; + return `Genotype ${genotype.gene}: ${genotype.positions.join(", ")}`; +}; diff --git a/src/util/scatterplotHelpers.js b/src/util/scatterplotHelpers.js index d4b10b766..1335df4f0 100644 --- a/src/util/scatterplotHelpers.js +++ b/src/util/scatterplotHelpers.js @@ -1,17 +1,25 @@ import { calcColorScale } from "./colorScale"; +import { makeGenotypeLabel, isColorByGenotype} from "./getGenotype"; -export function collectAvailableScatterVariables(colorings) { - // todo: genotype (special case) - // Note for implementation of genotype - be careful with calls to `calcColorScale` (e.g. from `validateScatterVariables`) - // as one of the side effects of that function is setting the genotype on nodes - const options = Object.keys(colorings) - .filter((key) => key!=="gt") +export function collectAvailableScatterVariables(colorings, colorBy) { + let options = Object.keys(colorings) .filter((key) => colorings[key].type!=="boolean") .map((key) => ({ value: key, label: colorings[key].title || key })); + + /* If colorBy is genotype, then we allow it to be an option, else we remove it */ + const genotypeLabel = makeGenotypeLabel(colorBy); + if (genotypeLabel) { + options.forEach((o) => { + if (o.value==="gt") o.label = genotypeLabel; + }); + } else { + options = options.filter((o) => o.value!=="gt"); + } options.unshift({value: "div", label: "Divergence"}); + return options; } @@ -21,7 +29,7 @@ export function collectAvailableScatterVariables(colorings) { export function validateScatterVariables(controls, metadata, tree, isClock) { const {distanceMeasure, colorBy } = controls; const existingScatterVariables = {...controls.scatterVariables}; - const availableOptions = collectAvailableScatterVariables(metadata.colorings); + const availableOptions = collectAvailableScatterVariables(metadata.colorings, controls.colorBy); const scatterVariables = {}; // default is to show branches, unless the existing state says otherwise scatterVariables.showBranches = Object.prototype.hasOwnProperty.call(existingScatterVariables, "showBranches") ? @@ -32,11 +40,11 @@ export function validateScatterVariables(controls, metadata, tree, isClock) { // we only validate the x & y values if we're _not_ in clock mode, as we don't use them there! if (!isClock) { // default X value is existing state, or the distanceMeasure. It should not be the existing y value (if that's set) - const xOption = _getFirstMatchingOption(availableOptions, [existingScatterVariables.x, distanceMeasure], existingScatterVariables.y); + const xOption = getFirstMatchingScatterVariable(availableOptions, [existingScatterVariables.x, distanceMeasure], existingScatterVariables.y); scatterVariables.x = xOption.value; scatterVariables.xLabel = xOption.label; // default Y value is similar, but we default to the colorBy (if available) - const yOption = _getFirstMatchingOption(availableOptions, [existingScatterVariables.y, colorBy], xOption.value); + const yOption = getFirstMatchingScatterVariable(availableOptions, [existingScatterVariables.y, colorBy], xOption.value); scatterVariables.y = yOption.value; scatterVariables.yLabel = yOption.label; for (const axis of ["x", "y"]) { @@ -53,11 +61,12 @@ export function validateScatterVariables(controls, metadata, tree, isClock) { * First scans through a list of values to try (`tryTheseFirst`) * Will not return an option whose key matches `notThisValue` */ -function _getFirstMatchingOption(options, tryTheseFirst, notThisValue) { +export function getFirstMatchingScatterVariable(options, tryTheseFirst, notThisValue) { const availableValues = options.map((opt) => opt.value); for (let i=0; i Date: Fri, 21 May 2021 19:29:09 +1200 Subject: [PATCH 4/5] Improve padding for categorical scatterplot variables This prevents nodes falling on the axis itself or at the very end of the grid, which was especially noticeable for traits with small domains. --- src/components/tree/phyloTree/layouts.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/components/tree/phyloTree/layouts.js b/src/components/tree/phyloTree/layouts.js index 2de7f6ff7..d4757301e 100644 --- a/src/components/tree/phyloTree/layouts.js +++ b/src/components/tree/phyloTree/layouts.js @@ -274,12 +274,12 @@ export const setDistance = function setDistance(distanceAttribute) { export const setScales = function setScales(margins) { if (this.layout==="scatter" && !this.scatterVariables.xContinuous) { - this.xScale = scalePoint().round(true).align(0.5); + this.xScale = scalePoint().round(false).align(0.5); } else { this.xScale = scaleLinear(); } if (this.layout==="scatter" && !this.scatterVariables.yContinuous) { - this.yScale = scalePoint().round(true).align(0.5); + this.yScale = scalePoint().round(false).align(0.5); } else { this.yScale = scaleLinear(); } @@ -384,6 +384,7 @@ export const mapToScreen = function mapToScreen() { } else { const seenValues = new Set(nodesInDomain.map((d) => d.x)); xDomain = this.scatterVariables.xDomain.filter((v) => seenValues.has(v)); + padCategoricalScales(xDomain, this.xScale); } if (this.layout!=="scatter" || this.scatterVariables.yContinuous) { @@ -403,6 +404,7 @@ export const mapToScreen = function mapToScreen() { } else { const seenValues = new Set(nodesInDomain.map((d) => d.y)); yDomain = this.scatterVariables.yDomain.filter((v) => seenValues.has(v)); + padCategoricalScales(yDomain, this.yScale); } /* Radial / Unrooted layouts need to be square since branch lengths @@ -505,3 +507,10 @@ export const mapToScreen = function mapToScreen() { } timerEnd("mapToScreen"); }; + +function padCategoricalScales(domain, scale) { + if (domain.length<=4) return scale.padding(0.4); + if (domain.length<=6) return scale.padding(0.3); + if (domain.length<=10) return scale.padding(0.2); + return scale.padding(0.1); +} From 898a387de488233eba70570ae15b39c52606be63 Mon Sep 17 00:00:00 2001 From: James Hadfield Date: Fri, 21 May 2021 20:18:08 +1200 Subject: [PATCH 5/5] Add jitter to categorical scatterplots --- src/components/tree/phyloTree/layouts.js | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/components/tree/phyloTree/layouts.js b/src/components/tree/phyloTree/layouts.js index d4757301e..081568379 100644 --- a/src/components/tree/phyloTree/layouts.js +++ b/src/components/tree/phyloTree/layouts.js @@ -438,6 +438,8 @@ export const mapToScreen = function mapToScreen() { // for scatterplots we do an additional iteration as some values may be missing // & we want to avoid rendering these if (this.layout==="scatter") { + if (!this.scatterVariables.yContinuous) jitter("y", this.yScale, this.nodes); + if (!this.scatterVariables.xContinuous) jitter("x", this.xScale, this.nodes); this.nodes.forEach((d) => { if (isNaN(d.xTip)) d.xTip = hiddenXPosition; if (isNaN(d.yTip)) d.yTip=hiddenYPosition; @@ -514,3 +516,25 @@ function padCategoricalScales(domain, scale) { if (domain.length<=10) return scale.padding(0.2); return scale.padding(0.1); } + +/** + * Add jitter to the already-computed node positions. + */ +function jitter(axis, scale, nodes) { + const step = scale.step(); + if (step < 50) return; // don't jitter if there's little space between bands + const rand = []; // pre-compute a small set of pseudo random numbers for speed + for (let i=1e2; i--;) { + rand.push((Math.random()-0.5)*step*0.5); // occupy 50% + } + 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++]; + if (j>=randLen) j=0; + if (!n.children) return; + for (const child of n.children) recurse(child); + } + recurse(nodes[0]); +}