Skip to content

Commit

Permalink
🎉 (slope) add tolerance / TAS-719 (#4205)
Browse files Browse the repository at this point in the history
Adds tolerance to slope charts.

If the original times of start and end value are too close together, then we don't show the slope. This is something that came out of discussion with Joe and Daniel, but now that I'm working on this, I don't think we need to be that strict? As long as the original start time < the original end time, we should be good? Curious what you think!

### Testing

Example charts:
- http://staging-site-slope-tolerance-viz/grapher/ratio-of-female-to-male-labor-force-participation-rates-slopes?time=2000..latest&country=AGO~OWID_WRL
- With forced relative mode (doesn't make much sense here, just for testing): http://staging-site-slope-tolerance-viz/grapher/ratio-of-female-to-male-labor-force-participation-rates-slopes?time=2000..latest&country=AGO~OWID_WRL&stackMode=relative
  • Loading branch information
sophiamersmann authored Dec 11, 2024
1 parent 4219a0b commit 93cd99a
Show file tree
Hide file tree
Showing 15 changed files with 154 additions and 58 deletions.
25 changes: 22 additions & 3 deletions packages/@ourworldindata/core-table/src/CoreTableColumns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -536,14 +536,16 @@ export abstract class AbstractCoreColumn<JS_TYPE extends PrimitiveType> {
// assumes table is sorted by time
@imemo get owidRows(): OwidVariableRow<JS_TYPE>[] {
const entities = this.allEntityNames
const times = this.originalTimes
const times = this.allTimes
const values = this.values
const originalTimes = this.originalTimes
const originalValues = this.originalValues
return range(0, times.length).map((index) => {
return range(0, originalTimes.length).map((index) => {
return omitUndefinedValues({
entityName: entities[index],
time: times[index],
value: values[index],
originalTime: originalTimes[index],
originalValue: originalValues[index],
})
})
Expand All @@ -562,6 +564,23 @@ export abstract class AbstractCoreColumn<JS_TYPE extends PrimitiveType> {
return map
}

// todo: remove? Should not be on CoreTable
@imemo get owidRowByEntityNameAndTime(): Map<
EntityName,
Map<Time, OwidVariableRow<JS_TYPE>>
> {
const valueByEntityNameAndTime = new Map<
EntityName,
Map<Time, OwidVariableRow<JS_TYPE>>
>()
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<
Expand All @@ -577,7 +596,7 @@ export abstract class AbstractCoreColumn<JS_TYPE extends PrimitiveType> {
valueByEntityNameAndTime.set(row.entityName, new Map())
valueByEntityNameAndTime
.get(row.entityName)!
.set(row.time, row.value)
.set(row.originalTime, row.value)
})
return valueByEntityNameAndTime
}
Expand Down
8 changes: 4 additions & 4 deletions packages/@ourworldindata/core-table/src/OwidTable.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ it("can parse data to Javascript data structures", () => {
table.get("Population").owidRows.forEach((row) => {
expect(typeof row.entityName).toBe("string")
expect(row.value).toBeGreaterThan(100)
expect(row.time).toBeGreaterThan(1999)
expect(row.originalTime).toBeGreaterThan(1999)
})
})

Expand Down Expand Up @@ -632,7 +632,7 @@ describe("tolerance", () => {
})
})

it("assigns originalTime as 'time' in owidRows", () => {
it("assigns originalTime as 'originalTime' in owidRows", () => {
const csv = `gdp,year,entityName,entityId,entityCode
1000,2019,USA,,
1001,2020,UK,,`
Expand All @@ -642,7 +642,7 @@ it("assigns originalTime as 'time' in owidRows", () => {
expect.not.arrayContaining([
expect.objectContaining({
entityName: "USA",
time: 2020,
originalTime: 2020,
value: 1000,
}),
])
Expand All @@ -651,7 +651,7 @@ it("assigns originalTime as 'time' in owidRows", () => {
expect.not.arrayContaining([
expect.objectContaining({
entityName: "UK",
time: 2019,
originalTime: 2019,
value: 1001,
}),
])
Expand Down
5 changes: 4 additions & 1 deletion packages/@ourworldindata/grapher/src/core/Grapher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -884,7 +884,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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ export class EntitySelector extends React.Component<{
const rows = column.owidRowsByEntityName.get(entityName) ?? []
searchableEntity[column.slug] = maxBy(
rows,
(row) => row.time
(row) => row.originalTime
)?.value
}

Expand Down
4 changes: 2 additions & 2 deletions packages/@ourworldindata/grapher/src/mapCharts/MapChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -346,12 +346,12 @@ export class MapChart

return mapColumn.owidRows
.map((row) => {
const { entityName, value, time } = row
const { entityName, value, originalTime } = row
const color = this.colorScale.getColor(value) || "red" // todo: color fix
if (!color) return undefined
return {
seriesName: entityName,
time,
time: originalTime,
value,
isSelected: selectionArray.selectedSet.has(entityName),
color,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export class MapSparkline extends React.Component<{
lineStrokeWidth: 2,
entityYearHighlight: {
entityName: this.manager.entityName,
year: this.manager.datum?.time,
year: this.manager.datum?.originalTime,
},
yAxisConfig: {
hideAxis: true,
Expand Down
6 changes: 3 additions & 3 deletions packages/@ourworldindata/grapher/src/mapCharts/MapTooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ export class MapTooltip
: targetTime?.toString()
const displayDatumTime =
timeColumn && datum
? timeColumn.formatValue(datum?.time)
: (datum?.time.toString() ?? "")
? timeColumn.formatValue(datum?.originalTime)
: (datum?.originalTime.toString() ?? "")
const valueColor: string | undefined = darkenColorForHighContrastText(
lineColorScale?.getColor(datum?.value) ?? "#333"
)
Expand All @@ -143,7 +143,7 @@ export class MapTooltip
const yColumn = this.mapTable.get(this.mapColumnSlug)

const targetNotice =
datum && datum.time !== targetTime ? displayTime : undefined
datum && datum.originalTime !== targetTime ? displayTime : undefined
const toleranceNotice = targetNotice
? {
icon: TooltipFooterIcon.notice,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,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()
})
Expand Down
126 changes: 98 additions & 28 deletions packages/@ourworldindata/grapher/src/slopeCharts/SlopeChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import { ColorSchemes } from "../color/ColorSchemes"
import { LineLabelSeries, LineLegend } from "../lineLegend/LineLegend"
import {
makeTooltipRoundingNotice,
makeTooltipToleranceNotice,
Tooltip,
TooltipState,
TooltipValueRange,
Expand Down Expand Up @@ -114,12 +115,20 @@ export class SlopeChart
if (this.isLogScale)
table = table.replaceNonPositiveCellsForLogScale(this.yColumnSlugs)

this.yColumnSlugs.forEach((slug) => {
table = table.interpolateColumnWithTolerance(slug)
})

return table
}

transformTableForSelection(table: OwidTable): OwidTable {
table = table.replaceNonNumericCellsWithErrorValues(this.yColumnSlugs)

this.yColumnSlugs.forEach((slug) => {
table = table.interpolateColumnWithTolerance(slug)
})

// if time selection is disabled, then filter all entities that
// don't have data for the current time period
if (!this.manager.hasTimeline && this.startTime !== this.endTime) {
Expand Down Expand Up @@ -276,10 +285,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,
Expand All @@ -299,16 +307,38 @@ export class SlopeChart
seriesName,
entityName,
color,
startValue,
endValue,
start,
end,
annotation,
}
}

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.
Expand Down Expand Up @@ -370,8 +400,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)
Expand Down Expand Up @@ -406,8 +436,8 @@ export class SlopeChart

@computed get allYValues(): number[] {
return this.series.flatMap((series) => [
series.startValue,
series.endValue,
series.start.value,
series.end.value,
])
}

Expand Down Expand Up @@ -522,13 +552,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,
}
})
}
Expand Down Expand Up @@ -660,17 +690,37 @@ export class SlopeChart
const { series } = target || {}
if (!series) return

const title = isRelativeMode
? `${series.seriesName}, ${formatColumn.formatTime(endTime)}`
: series.seriesName
const formatTime = (time: Time) => formatColumn.formatTime(time)

const timeRange = [startTime, endTime]
.map((t) => formatColumn.formatTime(t))
.join(" to ")
const actualStartTime = series.start.originalTime
const actualEndTime = series.end.originalTime
const timeRange = `${formatTime(actualStartTime)} to ${formatTime(actualEndTime)}`
const timeLabel = isRelativeMode
? `% change since ${formatColumn.formatTime(startTime)}`
? `% change between ${formatColumn.formatTime(actualStartTime)} and ${formatColumn.formatTime(actualEndTime)}`
: timeRange

const constructTargetYearForToleranceNotice = () => {
const isStartValueOriginal = series.start.originalTime === startTime
const isEndValueOriginal = series.end.originalTime === endTime

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 = series.column.roundsToSignificantFigures
? {
icon: TooltipFooterIcon.none,
Expand All @@ -680,11 +730,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 (
<Tooltip
Expand All @@ -695,8 +745,9 @@ export class SlopeChart
offsetX={20}
offsetY={-16}
style={{ maxWidth: "250px" }}
title={title}
title={series.seriesName}
subtitle={timeLabel}
subtitleFormat={targetYear ? "notice" : undefined}
dissolve={fading}
footer={footer}
dismiss={() => (this.tooltipState.target = null)}
Expand All @@ -707,16 +758,35 @@ export class SlopeChart
}

private makeMissingDataLabel(series: RawSlopeChartSeries): string {
const { seriesName } = series
const { seriesName, start, end } = series

const startTime = this.formatColumn.formatTime(this.startTime)
const endTime = this.formatColumn.formatTime(this.endTime)
if (series.startValue === undefined && series.endValue === undefined) {

// mention the start or end value if they're missing
if (start?.value === undefined && end?.value === undefined) {
return `${seriesName} (${startTime} & ${endTime})`
} else if (series.startValue === undefined) {
} else if (start?.value === undefined) {
return `${seriesName} (${startTime})`
} else if (series.endValue === undefined) {
} else if (end?.value === undefined) {
return `${seriesName} (${endTime})`
}

// if both values are given but the series shows up in the No Data
// section, then tolerance has been applied to one of the values
// in such a way that we decided not to render the slope after all
// (e.g. when the original times are too close to each other)
const isToleranceAppliedToStartValue =
start.originalTime !== this.startTime
const isToleranceAppliedToEndValue = end.originalTime !== this.endTime
if (isToleranceAppliedToStartValue && isToleranceAppliedToEndValue) {
return `${seriesName} (${startTime} & ${endTime})`
} else if (isToleranceAppliedToStartValue) {
return `${seriesName} (${startTime})`
} else if (isToleranceAppliedToEndValue) {
return `${seriesName} (${endTime})`
}

return seriesName
}

Expand Down
Loading

0 comments on commit 93cd99a

Please sign in to comment.