From 5a710a3bc2e20832218bd1f1e203ef310ecb1123 Mon Sep 17 00:00:00 2001 From: james hadfield Date: Tue, 27 Aug 2024 13:45:38 +1200 Subject: [PATCH 1/6] Allow ambiguous date parsing Modifies the utility function to optionally allow ambiguous dates such as YYYY-MM-XX or YYYY-XX-XX. Additionally error checking has been improved and the return value for any non-valid string date is now always `undefined`. Out of interest I timed the performance of (unambiguous) date parsing before / after this change and saw only a small change: before: 0.0021ms, after: 0.0023ms (~2 microseconds). Code used for timing: ```js window.timeit = (allowInvalidStrings = false) => { const n = 100_000; const values = Array.from(Array(n)).map(() => String(parseInt(String(2000 + Math.random()*20))) + "-" + String(parseInt(String(Math.random()*12))).padStart(2, '0') + "-" + String(parseInt(String(Math.random()*28))).padStart(2, '0') ) const start = performance.now(); values.forEach((v) => calendarToNumeric(v)) console.log(`Time taken ${(performance.now() - start)/n}ms per call`) } ``` --- src/util/dateHelpers.js | 49 ++++++++++++++++++++++++++++++++--------- test/dates.test.js | 47 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 11 deletions(-) diff --git a/src/util/dateHelpers.js b/src/util/dateHelpers.js index dd2d6b550..847f3b396 100644 --- a/src/util/dateHelpers.js +++ b/src/util/dateHelpers.js @@ -39,26 +39,53 @@ export const numericToCalendar = (numDate) => { }; /** - * Convert a calendar date to a numeric one. - * This function is meant to behave similarly to TreeTime's `numeric_date` - * as found in v0.7*. Note that for negative dates, i.e. BCE, no fraction - * in the year will be returned. + * Convert a YYYY-MM-DD string to a numeric date. This function is meant to + * behave similarly to TreeTime's `numeric_date` as found in v0.7*. For negative + * dates (i.e. BCE) we simply return the year (ignoring month / day). Ambiguity + * is optionally allowed in the form of YYYY-MM-XX or YYYY-XX-XX in which case + * the midpoint of the implied range is returned. All non compliant inputs + * return `undefined`. * @param {string} calDate in format YYYY-MM-DD - * @returns {float} YYYY.F, where F is the fraction of the year passed + * @param {boolean} ambiguity + * @returns {float|undefined} YYYY.F, where F is the fraction of the year passed */ -export const calendarToNumeric = (calDate) => { +export const calendarToNumeric = (calDate, ambiguity=false) => { + if (typeof calDate !== "string") return undefined; if (calDate[0]==='-') { - const pieces = calDate.substring(1).split('-'); - return -parseFloat(pieces[0]); + const d = -parseFloat(calDate.substring(1).split('-')[0]); + return isNaN(d) ? undefined : d; } - /* Beware: for `Date`, months are 0-indexed, days are 1-indexed */ - const [year, month, day] = calDate.split("-").map((n) => parseInt(n, 10)); + const fields = calDate.split("-"); + if (fields.length !== 3) return undefined; + const [year, month, day] = fields; + const [numYear, numMonth, numDay] = fields.map((d) => parseInt(d, 10)); + + if (calDate.includes("X")) { + if (!ambiguity) return undefined + if (year.includes("X")) return undefined; + if (month.includes("X")) { + if (isNaN(numYear) || month!=="XX" || day!=="XX") return undefined + return numYear + 0.5; + } + /* at this point 'day' includes 'X' */ + if (isNaN(numYear) || isNaN(numMonth) || day!=='XX') return undefined + const range = [ + _yearMonthDayToNumeric(numYear, numMonth, 1), + _yearMonthDayToNumeric(numMonth===12?numYear+1:numYear, numMonth===12?1:numMonth+1, 1) + ] + return range[0] + (range[1]-range[0])/2 + } + return _yearMonthDayToNumeric(numYear, numMonth, numDay) +}; + +function _yearMonthDayToNumeric(year,month,day) { const oneDayInMs = 86400000; // 1000 * 60 * 60 * 24 + /* Beware: for `Date`, months are 0-indexed, days are 1-indexed */ /* add on 1/2 day to let time represent noon (12h00) */ const elapsedDaysInYear = (Date.UTC(year, month-1, day) - Date.UTC(year, 0, 1)) / oneDayInMs + 0.5; const fracPart = elapsedDaysInYear / (isLeapYear(year) ? 366 : 365); return year + fracPart; -}; +} export const currentCalDate = () => dateToString(new Date()); diff --git a/test/dates.test.js b/test/dates.test.js index 463a57f56..900066507 100644 --- a/test/dates.test.js +++ b/test/dates.test.js @@ -93,3 +93,50 @@ test("dates are prettified as expected", () => { expect(prettifyDate("MONTH", "2020-01-01")).toStrictEqual("2020-Jan"); expect(prettifyDate("CENTURY", "-3000-01-01")).toStrictEqual("-3000"); // BCE }); + + +const ambiguousCalendarDates = { + "2024-XX-XX": 2024.5, + "2024-01-XX": 2024.044, + "2024-11-XX": 2024.88, + "2024-12-XX": 2024.96, +} + +test("calendarToNumeric doesn't allow ambiguous dates unless requested", () => { + for (const [calendarDate, _] of Object.entries(ambiguousCalendarDates)) { + expect(calendarToNumeric(calendarDate)).toBe(undefined) + } +}); + +test("calendarToNumeric allows ambiguous dates when requested", () => { + for (const [calendarDate, numericDate] of Object.entries(ambiguousCalendarDates)) { + expect(calendarToNumeric(calendarDate, true)).toBeCloseTo(numericDate); + } +}); + +const invalidUnambiguousCalendarDates = [ + "string", + "-string", + "2024-XX-01", + "2024-X-XX", + "2024-XX-XXX", + 2024.123, +] + +test("calendarToNumeric returns undefined for erroneous dates without ambiguity", () => { + for (const calendarDate of invalidUnambiguousCalendarDates) { + expect(calendarToNumeric(calendarDate)).toBe(undefined); + } +}); + +const invalidAmbiguousCalendarDates = [ + "2024-XX-01", + "2024-X-XX", + "2024-XX-XXX", +] + +test("calendarToNumeric returns undefined for erroneous dates with ambiguity", () => { + for (const calendarDate of invalidAmbiguousCalendarDates) { + expect(calendarToNumeric(calendarDate, true)).toBe(undefined); + } +}); From d9424347053b337100ee9810acbeb180667f3b14 Mon Sep 17 00:00:00 2001 From: james hadfield Date: Tue, 27 Aug 2024 14:30:08 +1200 Subject: [PATCH 2/6] Validate URL query provided dates Previously invalid dates would result in an invalid date range, often resulting in no tips selected and errors in the console. The resulting SVG errors due to NaNs may have resulted in the tree not being rendered correctly in some browsers. The error messages were influenced by code review comments on --- src/actions/recomputeReduxState.js | 74 +++++++++++++++++++++++------- 1 file changed, 57 insertions(+), 17 deletions(-) diff --git a/src/actions/recomputeReduxState.js b/src/actions/recomputeReduxState.js index e802da9fa..ff372ed34 100644 --- a/src/actions/recomputeReduxState.js +++ b/src/actions/recomputeReduxState.js @@ -92,13 +92,31 @@ const modifyStateViaURLQuery = (state, query) => { if (query.tl) { state["tipLabelKey"] = query.tl===strainSymbolUrlString ? strainSymbol : query.tl; } + if (query.dmin) { - state["dateMin"] = query.dmin; - state["dateMinNumeric"] = calendarToNumeric(query.dmin); + const dateNum = calendarToNumeric(query.dmin); + if (_validDate(dateNum, state.absoluteDateMinNumeric, state.absoluteDateMaxNumeric)) { + state["dateMin"] = query.dmin; + state["dateMinNumeric"] = dateNum; + } else { + console.error(`URL query "dmin=${query.dmin}" is invalid ${state.branchLengthsToDisplay==='divOnly'?'(the tree is not a timetree)':''}`); + delete query.dmin; + } } if (query.dmax) { - state["dateMax"] = query.dmax; - state["dateMaxNumeric"] = calendarToNumeric(query.dmax); + const dateNum = calendarToNumeric(query.dmax); + if (_validDate(dateNum, state.absoluteDateMinNumeric, state.absoluteDateMaxNumeric)) { + if ((query.dmin && dateNum <= state.dateMinNumeric)) { + console.error(`Requested "dmax=${query.dmax}" is earlier than "dmin=${query.dmin}", ignoring dmax.`); + delete query.dmax; + } else { + state["dateMax"] = query.dmax; + state["dateMaxNumeric"] = dateNum; + } + } else { + console.error(`URL query "dmax=${query.dmax}" is invalid ${state.branchLengthsToDisplay==='divOnly'?'(the tree is not a timetree)':''}`); + delete query.dmax; + } } /** Queries 's', 'gt', and 'f_' correspond to active filters */ @@ -115,21 +133,39 @@ const modifyStateViaURLQuery = (state, query) => { state.filters[genotypeSymbol] = decodeGenotypeFilters(query.gt||""); } + state.animationPlayPauseButton = "Play"; if (query.animate) { const params = query.animate.split(','); - // console.log("start animation!", params); - window.NEXTSTRAIN.animationStartPoint = calendarToNumeric(params[0]); - window.NEXTSTRAIN.animationEndPoint = calendarToNumeric(params[1]); - state.dateMin = params[0]; - state.dateMax = params[1]; - state.dateMinNumeric = calendarToNumeric(params[0]); - state.dateMaxNumeric = calendarToNumeric(params[1]); - state.mapAnimationShouldLoop = params[2] === "1"; - state.mapAnimationCumulative = params[3] === "1"; - state.mapAnimationDurationInMilliseconds = parseInt(params[4], 10); - state.animationPlayPauseButton = "Pause"; - } else { - state.animationPlayPauseButton = "Play"; + if (params.length!==5) { + console.error("Invalid 'animate' URL query (not enough fields)"); + delete query.animate; + } else if (state.branchLengthsToDisplay==='divOnly') { + console.error("Invalid 'animate' URL query (tree is not a timetree)"); + delete query.animate; + } else { + const [_dmin, _dminNum] = [params[0], calendarToNumeric(params[0])]; + const [_dmax, _dmaxNum] = [params[1], calendarToNumeric(params[1])]; + if ( + !_validDate(_dminNum, state.absoluteDateMinNumeric, state.absoluteDateMaxNumeric) || + !_validDate(_dmaxNum, state.absoluteDateMinNumeric, state.absoluteDateMaxNumeric) || + _dminNum >= _dmaxNum + ) { + console.error("Invalid 'animate' URL query (invalid date range)") + delete query.animate + } else { + window.NEXTSTRAIN.animationStartPoint = _dminNum; + window.NEXTSTRAIN.animationEndPoint = _dmaxNum; + state.dateMin = _dmin; + state.dateMax = _dmax; + state.dateMinNumeric = _dminNum; + state.dateMaxNumeric = _dmaxNum; + state.mapAnimationShouldLoop = params[2] === "1"; + state.mapAnimationCumulative = params[3] === "1"; + const duration = parseInt(params[4], 10); + state.mapAnimationDurationInMilliseconds = isNaN(duration) ? 30_000 : duration; + state.animationPlayPauseButton = "Pause"; + } + } } if (query.branchLabel) { state.selectedBranchLabel = query.branchLabel; @@ -171,6 +207,10 @@ const modifyStateViaURLQuery = (state, query) => { if (query.scatterX) state.scatterVariables.x = query.scatterX; if (query.scatterY) state.scatterVariables.y = query.scatterY; return state; + + function _validDate(dateNum, absoluteDateMinNumeric, absoluteDateMaxNumeric) { + return !(dateNum===undefined || dateNum > absoluteDateMaxNumeric || dateNum < absoluteDateMinNumeric); + } }; const restoreQueryableStateToDefaults = (state) => { From 47b2e4163196b5967308485508d0d5b03e232bdc Mon Sep 17 00:00:00 2001 From: james hadfield Date: Tue, 27 Aug 2024 14:38:40 +1200 Subject: [PATCH 3/6] Remove unused date_range metadata In it's current form it is never used, as the metadata handed to `modifyStateViaMetadata` is created by `createMetadataStateFromJSON` which will never include 'date_range'. Neither is it part of the schema This code was originally added in 2017 and has been refactored multiple times since then. --- src/actions/recomputeReduxState.js | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/src/actions/recomputeReduxState.js b/src/actions/recomputeReduxState.js index ff372ed34..e1ee0ae15 100644 --- a/src/actions/recomputeReduxState.js +++ b/src/actions/recomputeReduxState.js @@ -249,23 +249,6 @@ const restoreQueryableStateToDefaults = (state) => { }; const modifyStateViaMetadata = (state, metadata, genomeMap) => { - if (metadata.date_range) { - /* this may be useful if, e.g., one were to want to display an outbreak - from 2000-2005 (the default is the present day) */ - if (metadata.date_range.date_min) { - state["dateMin"] = metadata.date_range.date_min; - state["dateMinNumeric"] = calendarToNumeric(state["dateMin"]); - state["absoluteDateMin"] = metadata.date_range.date_min; - state["absoluteDateMinNumeric"] = calendarToNumeric(state["absoluteDateMin"]); - state["mapAnimationStartDate"] = metadata.date_range.date_min; - } - if (metadata.date_range.date_max) { - state["dateMax"] = metadata.date_range.date_max; - state["dateMaxNumeric"] = calendarToNumeric(state["dateMax"]); - state["absoluteDateMax"] = metadata.date_range.date_max; - state["absoluteDateMaxNumeric"] = calendarToNumeric(state["absoluteDateMax"]); - } - } if (metadata.analysisSlider) { state["analysisSlider"] = {key: metadata.analysisSlider, valid: false}; } From 53b4382326701554eb5fe55488aa7de8357b4267 Mon Sep 17 00:00:00 2001 From: james hadfield Date: Tue, 27 Aug 2024 10:30:15 +1200 Subject: [PATCH 4/6] refactor continuous/temporal scale into separate functions so that we can more easily make modifications to the temporal one in subsequent commits --- src/util/colorScale.js | 85 +++++++++++++++++++++++++----------------- 1 file changed, 51 insertions(+), 34 deletions(-) diff --git a/src/util/colorScale.js b/src/util/colorScale.js index ce8a6aa9b..0e745363b 100644 --- a/src/util/colorScale.js +++ b/src/util/colorScale.js @@ -48,9 +48,12 @@ export const calcColorScale = (colorBy, controls, tree, treeToo, metadata) => { ({legendValues, colorScale} = createScaleForGenotype(tree.nodes, treeToo?.nodes, genotype.aa)); domain = [...legendValues]; } else if (colorings && colorings[colorBy]) { - if (scaleType === "continuous" || scaleType==="temporal") { + if (scaleType === "temporal" || colorBy === "num_date") { ({continuous, colorScale, legendBounds, legendValues} = - createContinuousScale(colorBy, colorings[colorBy].scale, tree.nodes, treeTooNodes, scaleType==="temporal")); + createTemporalScale(colorBy, colorings[colorBy].scale, tree.nodes, treeTooNodes)); + } else if (scaleType === "continuous") { + ({continuous, colorScale, legendBounds, legendValues} = + createContinuousScale(colorBy, colorings[colorBy].scale, tree.nodes, treeTooNodes)); } else if (colorings[colorBy].scale) { /* scale set via JSON */ ({continuous, legendValues, colorScale} = createNonContinuousScaleFromProvidedScaleMap(colorBy, colorings[colorBy].scale, tree.nodes, treeTooNodes)); @@ -218,19 +221,10 @@ function createOrdinalScale(colorBy, t1nodes, t2nodes) { return {continuous, colorScale, legendValues, legendBounds}; } -function createContinuousScale(colorBy, providedScale, t1nodes, t2nodes, isTemporal) { - /* Note that a temporal scale is treated very similar to a continuous one... for the time being. - In the future it'd be nice to allow YYYY-MM-DD values, but that's for another PR (and comes - with its own complexities - what about -XX dates?) james june 2022 */ - // console.log("making a continuous color scale for ", colorBy); - if (colorBy==="num_date") { - /* before numeric scales were a definable type, num_date was specified as continuous */ - isTemporal = true; - } +function createContinuousScale(colorBy, providedScale, t1nodes, t2nodes) { + let minMax; - if (isTemporal) { - // empty - minMax not needed - } else if (colorBy==="lbi") { + if (colorBy==="lbi") { minMax = [0, 0.7]; /* TODO: this is for historical reasons, and we should switch to a provided scale */ } else { minMax = getMinMaxFromTree(t1nodes, t2nodes, colorBy); @@ -244,8 +238,46 @@ function createContinuousScale(colorBy, providedScale, t1nodes, t2nodes, isTempo if (anchorPoints) { domain = anchorPoints.map((pt) => pt[0]); range = anchorPoints.map((pt) => pt[1]); - } else if (isTemporal) { - /* we want the colorScale to "focus" on the tip dates, and be spaced according to sampling */ + } else { + range = colors[9]; + domain = genericDomain.map((d) => minMax[0] + d * (minMax[1] - minMax[0])); + } + const scale = scaleLinear().domain(domain).range(range); + + let legendValues; + if (colorBy==="lbi") { + /* TODO: this is for historical reasons, and we should switch to a provided scale */ + legendValues = [0, 0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7]; + } else { + const spread = minMax[1] - minMax[0]; + const dp = spread > 5 ? 2 : 3; + /* if legend values are identical (for the specified number of decimal places) then we + should filter them out */ + legendValues = genericDomain + .map((d) => parseFloat((minMax[0] + d*spread).toFixed(dp))) + .filter((el, idx, values) => values.indexOf(el)===idx); + } + // Hack to avoid a bug: https://github.com/nextstrain/auspice/issues/540 + if (Object.is(legendValues[0], -0)) legendValues[0] = 0; + + return { + continuous: true, + colorScale: (val) => isValueValid(val) ? scale(val) : unknownColor, + legendBounds: createLegendBounds(legendValues), + legendValues + }; +} + + +function createTemporalScale(colorBy, providedScale, t1nodes, t2nodes) { + + /* user-defined anchor points across the scale - note previously temporal scales could use this, + although I doubt any did */ + // const anchorPoints = _validateContinuousAnchorPoints(providedScale); + + /* construct a domain / range which "focuses" on the tip dates, and be spaced according to sampling */ + let domain, range; + { let rootDate = getTraitFromNode(t1nodes[0], colorBy); let vals = t1nodes.filter((n) => !n.hasChildren) .map((n) => getTraitFromNode(n, colorBy)); @@ -265,27 +297,12 @@ function createContinuousScale(colorBy, providedScale, t1nodes, t2nodes, isTempo domain.push(vals[vals.length-1]); domain = [...new Set(domain)]; /* filter to unique values only */ range = colors[domain.length]; /* use the right number of colours */ - } else { - range = colors[9]; - domain = genericDomain.map((d) => minMax[0] + d * (minMax[1] - minMax[0])); } + const scale = scaleLinear().domain(domain).range(range); - let legendValues; - if (isTemporal) { - legendValues = domain.slice(1); - } else if (colorBy==="lbi") { - /* TODO: this is for historical reasons, and we should switch to a provided scale */ - legendValues = [0, 0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7]; - } else { - const spread = minMax[1] - minMax[0]; - const dp = spread > 5 ? 2 : 3; - /* if legend values are identical (for the specified number of decimal places) then we - should filter them out */ - legendValues = genericDomain - .map((d) => parseFloat((minMax[0] + d*spread).toFixed(dp))) - .filter((el, idx, values) => values.indexOf(el)===idx); - } + const legendValues = domain.slice(1); + // Hack to avoid a bug: https://github.com/nextstrain/auspice/issues/540 if (Object.is(legendValues[0], -0)) legendValues[0] = 0; From c5207ae5fcd41dcb5afd63f042186284f954b130 Mon Sep 17 00:00:00 2001 From: james hadfield Date: Tue, 27 Aug 2024 15:24:08 +1200 Subject: [PATCH 5/6] Allow YYYY-MM-DD values in temporal color scale These values may be ambiguous (using "XX" notation). There is no ability to provide a custom scale / legend for temporal scales, with errors printed to the console if these are attempted. --- src/util/colorHelpers.js | 17 +++++++++++++++++ src/util/colorScale.js | 24 ++++++++++++++++++------ src/util/tipRadiusHelpers.ts | 7 +++++-- 3 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/util/colorHelpers.js b/src/util/colorHelpers.js index d202c2bdd..8885e3a40 100644 --- a/src/util/colorHelpers.js +++ b/src/util/colorHelpers.js @@ -4,6 +4,7 @@ import scalePow from "d3-scale/src/pow"; import { isColorByGenotype, decodeColorByGenotype } from "./getGenotype"; import { getTraitFromNode } from "./treeMiscHelpers"; import { isValueValid } from "./globals"; +import { calendarToNumeric } from "./dateHelpers"; /** * Average over the visible colours for a given location @@ -147,3 +148,19 @@ export const getColorByTitle = (colorings, colorBy) => { return colorings[colorBy] === undefined ? "" : colorings[colorBy].title; }; + +/** + * We allow values (on nodes) to be encoded as numeric dates (2021.123) or + * YYYY-MM-DD strings. This helper function handles this flexibility and + * translates any provided value to either a number or undefined. + */ +export function numDate(value) { + switch (typeof value) { + case "number": + return value; + case "string": + return calendarToNumeric(value, true); // allow XX ambiguity + default: + return undefined; + } +} \ No newline at end of file diff --git a/src/util/colorScale.js b/src/util/colorScale.js index 0e745363b..76c4372a4 100644 --- a/src/util/colorScale.js +++ b/src/util/colorScale.js @@ -5,7 +5,7 @@ import { rgb } from "d3-color"; import { interpolateHcl } from "d3-interpolate"; import { genericDomain, colors, genotypeColors, isValueValid, NODE_VISIBLE } from "./globals"; import { countTraitsAcrossTree } from "./treeCountingHelpers"; -import { getExtraVals } from "./colorHelpers"; +import { getExtraVals, numDate } from "./colorHelpers"; import { isColorByGenotype, decodeColorByGenotype } from "./getGenotype"; import { setGenotype, orderOfGenotypeAppearance } from "./setGenotype"; import { getTraitFromNode } from "./treeMiscHelpers"; @@ -274,19 +274,22 @@ function createTemporalScale(colorBy, providedScale, t1nodes, t2nodes) { /* user-defined anchor points across the scale - note previously temporal scales could use this, although I doubt any did */ // const anchorPoints = _validateContinuousAnchorPoints(providedScale); + if (providedScale) { + console.error("Auspice currently doesn't allow a JSON-provided 'scale' for temporal colorings"); + } /* construct a domain / range which "focuses" on the tip dates, and be spaced according to sampling */ let domain, range; { - let rootDate = getTraitFromNode(t1nodes[0], colorBy); + let rootDate = numDate(getTraitFromNode(t1nodes[0], colorBy)); let vals = t1nodes.filter((n) => !n.hasChildren) - .map((n) => getTraitFromNode(n, colorBy)); + .map((n) => numDate(getTraitFromNode(n, colorBy))); if (t2nodes) { - const treeTooRootDate = getTraitFromNode(t2nodes[0], colorBy); + const treeTooRootDate = numDate(getTraitFromNode(t2nodes[0], colorBy)); if (treeTooRootDate < rootDate) rootDate = treeTooRootDate; vals.concat( t2nodes.filter((n) => !n.hasChildren) - .map((n) => getTraitFromNode(n, colorBy)) + .map((n) => numDate(getTraitFromNode(n, colorBy))) ); } vals = vals.sort(); @@ -306,9 +309,14 @@ function createTemporalScale(colorBy, providedScale, t1nodes, t2nodes) { // Hack to avoid a bug: https://github.com/nextstrain/auspice/issues/540 if (Object.is(legendValues[0], -0)) legendValues[0] = 0; + const colorScale = (val) => { + const d = numDate(val); + return d===undefined ? unknownColor : scale(d); + }; + return { continuous: true, - colorScale: (val) => isValueValid(val) ? scale(val) : unknownColor, + colorScale, legendBounds: createLegendBounds(legendValues), legendValues }; @@ -457,6 +465,10 @@ function _validateContinuousAnchorPoints(providedScale) { */ function parseUserProvidedLegendData(providedLegend, currentLegendValues, scaleType) { if (!Array.isArray(providedLegend)) return false; + if (scaleType==='temporal') { + console.error("Auspice currently doesn't allow a JSON-provided 'legend' for temporal colorings"); + return false; + } const data = scaleType==="continuous" ? providedLegend.filter((d) => typeof d.value === "number") : // continuous scales _must_ have numeric stops diff --git a/src/util/tipRadiusHelpers.ts b/src/util/tipRadiusHelpers.ts index 7838af958..df612fc27 100644 --- a/src/util/tipRadiusHelpers.ts +++ b/src/util/tipRadiusHelpers.ts @@ -1,5 +1,5 @@ import { tipRadius, tipRadiusOnLegendMatch } from "./globals"; -import { getTipColorAttribute } from "./colorHelpers"; +import { getTipColorAttribute, numDate } from "./colorHelpers"; import { getTraitFromNode } from "./treeMiscHelpers"; /** @@ -13,7 +13,10 @@ import { getTraitFromNode } from "./treeMiscHelpers"; * @returns bool */ export const determineLegendMatch = (selectedLegendItem: (string|number), node:any, colorScale:any) => { - const nodeAttr = getTipColorAttribute(node, colorScale); + let nodeAttr = getTipColorAttribute(node, colorScale); + if (colorScale.scaleType === 'temporal') { + nodeAttr = numDate(nodeAttr); + } if (colorScale.continuous) { if (selectedLegendItem === colorScale.legendValues[0] && nodeAttr===colorScale.legendBounds[selectedLegendItem][0]) { return true; From 6cad3398d17ee33b98372b3c39d597e4e0e156b9 Mon Sep 17 00:00:00 2001 From: james hadfield Date: Tue, 27 Aug 2024 15:47:52 +1200 Subject: [PATCH 6/6] Allow anchor points in temporal scales Anchor points can be YYYY-MM-DD or numeric dates. --- src/util/colorScale.js | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/util/colorScale.js b/src/util/colorScale.js index 76c4372a4..14a78e8eb 100644 --- a/src/util/colorScale.js +++ b/src/util/colorScale.js @@ -231,7 +231,7 @@ function createContinuousScale(colorBy, providedScale, t1nodes, t2nodes) { } /* user-defined anchor points across the scale */ - const anchorPoints = _validateContinuousAnchorPoints(providedScale); + const anchorPoints = _validateAnchorPoints(providedScale, (val) => typeof val==="number"); /* make the continuous scale */ let domain, range; @@ -271,16 +271,13 @@ function createContinuousScale(colorBy, providedScale, t1nodes, t2nodes) { function createTemporalScale(colorBy, providedScale, t1nodes, t2nodes) { - /* user-defined anchor points across the scale - note previously temporal scales could use this, - although I doubt any did */ - // const anchorPoints = _validateContinuousAnchorPoints(providedScale); - if (providedScale) { - console.error("Auspice currently doesn't allow a JSON-provided 'scale' for temporal colorings"); - } - - /* construct a domain / range which "focuses" on the tip dates, and be spaced according to sampling */ let domain, range; - { + const anchorPoints = _validateAnchorPoints(providedScale, (val) => numDate(val)!==undefined); + if (anchorPoints) { + domain = anchorPoints.map((pt) => numDate(pt[0])); + range = anchorPoints.map((pt) => pt[1]); + } else { + /* construct a domain / range which "focuses" on the tip dates, and be spaced according to sampling */ let rootDate = numDate(getTraitFromNode(t1nodes[0], colorBy)); let vals = t1nodes.filter((n) => !n.hasChildren) .map((n) => numDate(getTraitFromNode(n, colorBy))); @@ -304,7 +301,7 @@ function createTemporalScale(colorBy, providedScale, t1nodes, t2nodes) { const scale = scaleLinear().domain(domain).range(range); - const legendValues = domain.slice(1); + const legendValues = anchorPoints ? domain.slice() : domain.slice(1); // Hack to avoid a bug: https://github.com/nextstrain/auspice/issues/540 if (Object.is(legendValues[0], -0)) legendValues[0] = 0; @@ -437,11 +434,11 @@ function createLegendBounds(legendValues) { return legendBounds; } -function _validateContinuousAnchorPoints(providedScale) { +function _validateAnchorPoints(providedScale, validator) { if (!Array.isArray(providedScale)) return false; const ap = providedScale.filter((item) => Array.isArray(item) && item.length===2 && - typeof item[0]==="number" && // idx0 is the numerical value to anchor against + validator(item[0]) && typeof item[1]==="string" && item[1].match(/#[0-9A-Fa-f]{6}/) // schema demands full-length colour hexes ); if (ap.length<2) return false; // need at least 2 valid points @@ -466,7 +463,8 @@ function _validateContinuousAnchorPoints(providedScale) { function parseUserProvidedLegendData(providedLegend, currentLegendValues, scaleType) { if (!Array.isArray(providedLegend)) return false; if (scaleType==='temporal') { - console.error("Auspice currently doesn't allow a JSON-provided 'legend' for temporal colorings"); + console.error("Auspice currently doesn't allow a JSON-provided 'legend' for temporal colorings, "+ + "however all provided 'scale' entries will be shown in the legend"); return false; }