From 680b619d025717f1c4ee8bb166d304280eaab429 Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Wed, 27 Nov 2024 15:52:33 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20(slope)=20add=20tolerance?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../core-table/src/CoreTableColumns.ts | 17 +++ .../grapher/src/core/Grapher.tsx | 5 +- .../src/slopeCharts/SlopeChart.test.ts | 3 +- .../grapher/src/slopeCharts/SlopeChart.tsx | 117 ++++++++++++++---- .../src/slopeCharts/SlopeChartConstants.ts | 14 +-- .../grapher/src/tooltip/TooltipContents.tsx | 8 +- 6 files changed, 128 insertions(+), 36 deletions(-) diff --git a/packages/@ourworldindata/core-table/src/CoreTableColumns.ts b/packages/@ourworldindata/core-table/src/CoreTableColumns.ts index 89014482505..64b355d867e 100644 --- a/packages/@ourworldindata/core-table/src/CoreTableColumns.ts +++ b/packages/@ourworldindata/core-table/src/CoreTableColumns.ts @@ -564,6 +564,23 @@ export abstract class AbstractCoreColumn { return map } + // todo: remove? Should not be on CoreTable + @imemo get owidRowByEntityNameAndTime(): Map< + EntityName, + Map> + > { + const valueByEntityNameAndTime = new Map< + EntityName, + Map> + >() + this.owidRows.forEach((row) => { + if (!valueByEntityNameAndTime.has(row.entityName)) + valueByEntityNameAndTime.set(row.entityName, new Map()) + valueByEntityNameAndTime.get(row.entityName)!.set(row.time, row) + }) + return valueByEntityNameAndTime + } + // todo: remove? Should not be on CoreTable // NOTE: this uses the original times, so any tolerance is effectively unapplied. @imemo get valueByEntityNameAndOriginalTime(): Map< diff --git a/packages/@ourworldindata/grapher/src/core/Grapher.tsx b/packages/@ourworldindata/grapher/src/core/Grapher.tsx index e5166936d08..5225854352b 100644 --- a/packages/@ourworldindata/grapher/src/core/Grapher.tsx +++ b/packages/@ourworldindata/grapher/src/core/Grapher.tsx @@ -898,7 +898,10 @@ export class Grapher ) if (this.isOnSlopeChartTab) - return table.filterByTargetTimes([startTime, endTime]) + return table.filterByTargetTimes( + [startTime, endTime], + table.get(this.yColumnSlugs[0]).tolerance + ) return table.filterByTimeRange(startTime, endTime) } diff --git a/packages/@ourworldindata/grapher/src/slopeCharts/SlopeChart.test.ts b/packages/@ourworldindata/grapher/src/slopeCharts/SlopeChart.test.ts index 660f64c2953..02a2bd75f00 100755 --- a/packages/@ourworldindata/grapher/src/slopeCharts/SlopeChart.test.ts +++ b/packages/@ourworldindata/grapher/src/slopeCharts/SlopeChart.test.ts @@ -39,7 +39,8 @@ it("filters non-numeric values", () => { expect(chart.series.length).toEqual(1) expect( chart.series.every( - (series) => isNumber(series.startValue) && isNumber(series.endValue) + (series) => + isNumber(series.start.value) && isNumber(series.end.value) ) ).toBeTruthy() }) diff --git a/packages/@ourworldindata/grapher/src/slopeCharts/SlopeChart.tsx b/packages/@ourworldindata/grapher/src/slopeCharts/SlopeChart.tsx index 60e49f80593..694877a4251 100644 --- a/packages/@ourworldindata/grapher/src/slopeCharts/SlopeChart.tsx +++ b/packages/@ourworldindata/grapher/src/slopeCharts/SlopeChart.tsx @@ -60,6 +60,7 @@ import { ColorSchemes } from "../color/ColorSchemes" import { LineLabelSeries, LineLegend } from "../lineLegend/LineLegend" import { makeTooltipRoundingNotice, + makeTooltipToleranceNotice, Tooltip, TooltipState, TooltipValueRange, @@ -113,6 +114,10 @@ export class SlopeChart if (this.isLogScale) table = table.replaceNonPositiveCellsForLogScale(this.yColumnSlugs) + this.yColumnSlugs.forEach((slug) => { + table = table.interpolateColumnWithTolerance(slug) + }) + return table } @@ -120,9 +125,17 @@ export class SlopeChart // if entities with partial data are not plotted, // make sure they don't show up in the entity selector if (this.missingDataStrategy === MissingDataStrategy.hide) { - table = table - .replaceNonNumericCellsWithErrorValues(this.yColumnSlugs) - .dropEntitiesThatHaveNoDataInSomeColumn(this.yColumnSlugs) + table = table.replaceNonNumericCellsWithErrorValues( + this.yColumnSlugs + ) + + this.yColumnSlugs.forEach((slug) => { + table = table.interpolateColumnWithTolerance(slug) + }) + + table = table.dropEntitiesThatHaveNoDataInSomeColumn( + this.yColumnSlugs + ) } return table @@ -263,10 +276,9 @@ export class SlopeChart canSelectMultipleEntities, }) - const valueByTime = - column.valueByEntityNameAndOriginalTime.get(entityName) - const startValue = valueByTime?.get(startTime) - const endValue = valueByTime?.get(endTime) + const owidRowByTime = column.owidRowByEntityNameAndTime.get(entityName) + const start = owidRowByTime?.get(startTime) + const end = owidRowByTime?.get(endTime) const colorKey = getColorKey({ entityName, @@ -282,11 +294,12 @@ export class SlopeChart ) return { + column, seriesName, entityName, color, - startValue, - endValue, + start, + end, annotation, } } @@ -294,7 +307,29 @@ export class SlopeChart private isSeriesValid( series: RawSlopeChartSeries ): series is SlopeChartSeries { - return series.startValue !== undefined && series.endValue !== undefined + const { + start, + end, + column: { tolerance }, + } = series + + // if the start or end value is missing, we can't draw the slope + if (start?.value === undefined || end?.value === undefined) return false + + // sanity check (might happen if tolerance is enabled) + if (start.originalTime >= end.originalTime) return false + + const isToleranceAppliedToStartValue = + start.originalTime !== this.startTime + const isToleranceAppliedToEndValue = end.originalTime !== this.endTime + + // if tolerance has been applied to one of the values, then we require + // a minimal distance between the original times + if (isToleranceAppliedToStartValue || isToleranceAppliedToEndValue) { + return end.originalTime - start.originalTime >= tolerance + } + + return true } // Usually we drop rows with missing data in the transformTable function. @@ -356,8 +391,8 @@ export class SlopeChart const { yAxis, startX, endX } = this return this.series.map((series) => { - const startY = yAxis.place(series.startValue) - const endY = yAxis.place(series.endValue) + const startY = yAxis.place(series.start.value) + const endY = yAxis.place(series.end.value) const startPoint = new PointVector(startX, startY) const endPoint = new PointVector(endX, endY) @@ -381,8 +416,8 @@ export class SlopeChart @computed private get allValues(): number[] { return this.series.flatMap((series) => [ - series.startValue, - series.endValue, + series.start.value, + series.end.value, ]) } @@ -499,13 +534,13 @@ export class SlopeChart // used in LineLegend @computed get labelSeries(): LineLabelSeries[] { return this.series.map((series) => { - const { seriesName, color, endValue, annotation } = series + const { seriesName, color, end, annotation } = series return { color, seriesName, label: seriesName, annotation, - yValue: endValue, + yValue: end.value, } }) } @@ -637,13 +672,22 @@ export class SlopeChart const { series } = target || {} if (!series) return + const formatTime = (time: Time) => formatColumn.formatTime(time) + const title = isRelativeMode ? `${series.seriesName}, ${formatColumn.formatTime(endTime)}` : series.seriesName - const timeRange = [startTime, endTime] - .map((t) => formatColumn.formatTime(t)) - .join(" to ") + const isStartValueOriginal = series.start.originalTime === startTime + const isEndValueOriginal = series.end.originalTime === endTime + const actualStartTime = isStartValueOriginal + ? startTime + : series.start.originalTime + const actualEndTime = isEndValueOriginal + ? endTime + : series.end.originalTime + + const timeRange = `${formatTime(actualStartTime)} to ${formatTime(actualEndTime)}` const timeLabel = isRelativeMode ? `% change since ${formatColumn.formatTime(startTime)}` : timeRange @@ -663,6 +707,25 @@ export class SlopeChart ) ) + const constructTargetYearForToleranceNotice = () => { + if (!isStartValueOriginal && !isEndValueOriginal) { + return `${formatTime(startTime)} and ${formatTime(endTime)}` + } else if (!isStartValueOriginal) { + return formatTime(startTime) + } else if (!isEndValueOriginal) { + return formatTime(endTime) + } else { + return undefined + } + } + + const targetYear = constructTargetYearForToleranceNotice() + const toleranceNotice = targetYear + ? { + icon: TooltipFooterIcon.notice, + text: makeTooltipToleranceNotice(targetYear), + } + : undefined const roundingNotice = anyRoundedToSigFigs ? { icon: allRoundedToSigFigs @@ -673,11 +736,11 @@ export class SlopeChart }), } : undefined - const footer = excludeUndefined([roundingNotice]) + const footer = excludeUndefined([toleranceNotice, roundingNotice]) const values = isRelativeMode - ? [series.endValue] - : [series.startValue, series.endValue] + ? [series.end.value] + : [series.start.value, series.end.value] return ( (this.tooltipState.target = null)} @@ -703,11 +767,14 @@ export class SlopeChart const { seriesName } = series const startTime = this.formatColumn.formatTime(this.startTime) const endTime = this.formatColumn.formatTime(this.endTime) - if (series.startValue === undefined && series.endValue === undefined) { + if ( + series.start?.value === undefined && + series.end?.value === undefined + ) { return `${seriesName} (${startTime} & ${endTime})` - } else if (series.startValue === undefined) { + } else if (series.start?.value === undefined) { return `${seriesName} (${startTime})` - } else if (series.endValue === undefined) { + } else if (series.end?.value === undefined) { return `${seriesName} (${endTime})` } return seriesName diff --git a/packages/@ourworldindata/grapher/src/slopeCharts/SlopeChartConstants.ts b/packages/@ourworldindata/grapher/src/slopeCharts/SlopeChartConstants.ts index 544410718b7..a976bed8e2c 100644 --- a/packages/@ourworldindata/grapher/src/slopeCharts/SlopeChartConstants.ts +++ b/packages/@ourworldindata/grapher/src/slopeCharts/SlopeChartConstants.ts @@ -1,17 +1,17 @@ -import { EntityName, PartialBy, PointVector } from "@ourworldindata/utils" +import { PartialBy, PointVector } from "@ourworldindata/utils" +import { EntityName, OwidVariableRow } from "@ourworldindata/types" import { ChartSeries } from "../chart/ChartInterface" +import { CoreColumn } from "@ourworldindata/core-table" export interface SlopeChartSeries extends ChartSeries { + column: CoreColumn entityName: EntityName - startValue: number - endValue: number + start: Pick, "value" | "originalTime"> + end: Pick, "value" | "originalTime"> annotation?: string } -export type RawSlopeChartSeries = PartialBy< - SlopeChartSeries, - "startValue" | "endValue" -> +export type RawSlopeChartSeries = PartialBy export interface PlacedSlopeChartSeries extends SlopeChartSeries { startPoint: PointVector diff --git a/packages/@ourworldindata/grapher/src/tooltip/TooltipContents.tsx b/packages/@ourworldindata/grapher/src/tooltip/TooltipContents.tsx index 3078fd31d53..a3a8b7f0f1d 100644 --- a/packages/@ourworldindata/grapher/src/tooltip/TooltipContents.tsx +++ b/packages/@ourworldindata/grapher/src/tooltip/TooltipContents.tsx @@ -337,8 +337,12 @@ export function IconCircledS({ ) } -export function makeTooltipToleranceNotice(targetYear: string): string { - return `Data not available for ${targetYear}. Showing closest available data point instead` +export function makeTooltipToleranceNotice( + targetYear: string, + { plural }: { plural: boolean } = { plural: false } +): string { + const dataPoint = plural ? "data points" : "data point" + return `Data not available for ${targetYear}. Showing closest available ${dataPoint} instead` } export function makeTooltipRoundingNotice(