From e79f7762dad3f887997adad27e083fe7ecdb13ec Mon Sep 17 00:00:00 2001 From: Daniel Bachler Date: Mon, 23 Dec 2024 20:11:08 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A8=20make=20sure=20to=20add=20selecte?= =?UTF-8?q?d=20entity=20colors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../grapher/src/core/Grapher.tsx | 23 +++-- .../src/core/LegacyToOwidTable.test.ts | 74 ++++++++-------- .../grapher/src/core/LegacyToOwidTable.ts | 86 ++++++++++--------- 3 files changed, 102 insertions(+), 81 deletions(-) diff --git a/packages/@ourworldindata/grapher/src/core/Grapher.tsx b/packages/@ourworldindata/grapher/src/core/Grapher.tsx index 80fcfed7a4a..81f3ab24f9a 100644 --- a/packages/@ourworldindata/grapher/src/core/Grapher.tsx +++ b/packages/@ourworldindata/grapher/src/core/Grapher.tsx @@ -192,7 +192,11 @@ import { DefaultChartClass, } from "../chart/ChartTypeMap" import { Entity, SelectionArray } from "../selection/SelectionArray" -import { legacyToOwidTableAndDimensions } from "./LegacyToOwidTable" +import { + addSelectedEntityColorsToTable, + computeActualDimensions, + legacyToOwidTableAndDimensions, +} from "./LegacyToOwidTable" import { ScatterPlotManager } from "../scatterCharts/ScatterPlotChartConstants" import { autoDetectSeriesStrategy, @@ -1157,18 +1161,27 @@ export class Grapher // TODO grapher model: switch this to downloading multiple data and metadata files const startMark = performance.now() - const { dimensions, table } = legacyToOwidTableAndDimensions( + const table = legacyToOwidTableAndDimensions( json, - legacyConfig + legacyConfig.dimensions ?? [] ) + const tableWithColors = legacyConfig.selectedEntityColors + ? addSelectedEntityColorsToTable( + table, + legacyConfig.selectedEntityColors + ) + : table + const dimensions = legacyConfig.dimensions + ? computeActualDimensions(legacyConfig.dimensions) + : [] this.createPerformanceMeasurement( "legacyToOwidTableAndDimensions", startMark ) if (inputTableTransformer) - this.inputTable = inputTableTransformer(table) - else this.inputTable = table + this.inputTable = inputTableTransformer(tableWithColors) + else this.inputTable = tableWithColors // We need to reset the dimensions because some of them may have changed slugs in the legacy // transformation (can happen when columns use targetTime) diff --git a/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.test.ts b/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.test.ts index c7cac1c958b..2766a107bbc 100755 --- a/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.test.ts +++ b/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.test.ts @@ -8,7 +8,10 @@ import { LegacyGrapherInterface, } from "@ourworldindata/types" import { ColumnTypeMap, ErrorValueTypes } from "@ourworldindata/core-table" -import { legacyToOwidTableAndDimensions } from "./LegacyToOwidTable" +import { + addSelectedEntityColorsToTable, + legacyToOwidTableAndDimensions, +} from "./LegacyToOwidTable" import { MultipleOwidVariableDataDimensionsMap, OwidVariableDataMetadataDimensions, @@ -43,9 +46,9 @@ describe(legacyToOwidTableAndDimensions, () => { } it("contains the standard entity columns", () => { - const { table } = legacyToOwidTableAndDimensions( + const table = legacyToOwidTableAndDimensions( legacyVariableConfig, - legacyGrapherConfig + legacyGrapherConfig.dimensions ?? [] ) expect(table.columnSlugs).toEqual( expect.arrayContaining( @@ -62,27 +65,22 @@ describe(legacyToOwidTableAndDimensions, () => { describe("conversionFactor", () => { it("applies the more specific chart-level conversionFactor", () => { - const { table } = legacyToOwidTableAndDimensions( - legacyVariableConfig, + const table = legacyToOwidTableAndDimensions(legacyVariableConfig, [ { - dimensions: [ - { - variableId: 2, - display: { conversionFactor: 10 }, - property: DimensionProperty.y, - }, - ], - } - ) + variableId: 2, + display: { conversionFactor: 10 }, + property: DimensionProperty.y, + }, + ]) // Apply the chart-level conversionFactor (10) expect(table.rows[0]["2"]).toEqual(80) }) it("applies the more variable-level conversionFactor if a chart-level one is not present", () => { - const { table } = legacyToOwidTableAndDimensions( + const table = legacyToOwidTableAndDimensions( legacyVariableConfig, - legacyGrapherConfig + legacyGrapherConfig.dimensions ?? [] ) // Apply the variable-level conversionFactor (100) @@ -172,9 +170,9 @@ describe(legacyToOwidTableAndDimensions, () => { ], } - const { table } = legacyToOwidTableAndDimensions( + const table = legacyToOwidTableAndDimensions( legacyVariableConfig, - legacyGrapherConfig + legacyGrapherConfig.dimensions ?? [] ) it("leaves ErrorValues when there were no values to join to", () => { @@ -315,9 +313,9 @@ describe(legacyToOwidTableAndDimensions, () => { ], } - const { table } = legacyToOwidTableAndDimensions( + const table = legacyToOwidTableAndDimensions( legacyVariableConfig, - legacyGrapherConfig + legacyGrapherConfig.dimensions ?? [] ) it("shifts values in days array when zeroDay is is not EPOCH_DATE", () => { @@ -438,9 +436,9 @@ describe(legacyToOwidTableAndDimensions, () => { ], } - const { table } = legacyToOwidTableAndDimensions( + const table = legacyToOwidTableAndDimensions( legacyVariableConfig, - legacyGrapherConfig + legacyGrapherConfig.dimensions ?? [] ) it("duplicates 'day' column into 'time'", () => { @@ -469,9 +467,9 @@ describe(legacyToOwidTableAndDimensions, () => { chartTypes: [GRAPHER_CHART_TYPES.ScatterPlot], } - const { table } = legacyToOwidTableAndDimensions( + const table = legacyToOwidTableAndDimensions( legacyVariableConfig, - scatterLegacyGrapherConfig + scatterLegacyGrapherConfig.dimensions ?? [] ) expect(table.rows.length).toEqual(3) @@ -627,9 +625,9 @@ describe("variables with mixed days & years with missing overlap and multiple po ], } - const { table } = legacyToOwidTableAndDimensions( + const table = legacyToOwidTableAndDimensions( legacyVariableConfig, - legacyGrapherConfig + legacyGrapherConfig.dimensions ?? [] ) it("duplicates 'day' column into 'time'", () => { @@ -646,9 +644,9 @@ describe("variables with mixed days & years with missing overlap and multiple po describe("join behaviour without target times is sane", () => { it("creates a sane table join", () => { - const { table } = legacyToOwidTableAndDimensions( + const table = legacyToOwidTableAndDimensions( legacyVariableConfig, - legacyGrapherConfig + legacyGrapherConfig.dimensions ?? [] ) // A sane join between years and days would create 5 days for the given input @@ -764,10 +762,18 @@ const getLegacyGrapherConfig = (): Partial => { } describe("creating a table from legacy", () => { - const table = legacyToOwidTableAndDimensions(getOwidVarSet(), { + const config = { ...getLegacyGrapherConfig(), selectedEntityColors: { "Cape Verde": "blue" }, - }).table + } + const table = addSelectedEntityColorsToTable( + legacyToOwidTableAndDimensions( + getOwidVarSet(), + config.dimensions ?? [] + ), + config.selectedEntityColors + ) + const name = "Prevalence of wasting, weight for height (% of children under 5)" @@ -802,8 +808,8 @@ describe("creating a table from legacy", () => { expect( legacyToOwidTableAndDimensions( varSet, - getLegacyGrapherConfig() - ).table.get("3512")!.values + getLegacyGrapherConfig().dimensions ?? [] + ).get("3512")!.values ).toEqual([550, 420, 1260]) }) @@ -825,8 +831,8 @@ Papua New Guinea,PNG,1983,5.5` varSet.get(3512)!.metadata.nonRedistributable = true const columnDef = legacyToOwidTableAndDimensions( varSet, - getLegacyGrapherConfig() - ).table.get("3512").def as OwidColumnDef + getLegacyGrapherConfig().dimensions ?? [] + ).get("3512").def as OwidColumnDef expect(columnDef.nonRedistributable).toEqual(true) }) }) diff --git a/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.ts b/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.ts index 56039ca16b6..79b4fa25e8f 100644 --- a/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.ts +++ b/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.ts @@ -1,15 +1,14 @@ // todo: Remove this file when we've migrated OWID data and OWID charts to next version import { - GRAPHER_CHART_TYPES, ColumnTypeNames, CoreColumnDef, StandardOwidColumnDefs, OwidTableSlugs, OwidColumnDef, - LegacyGrapherInterface, OwidVariableDimensions, OwidVariableDataMetadataDimensions, + EntityId, } from "@ourworldindata/types" import { OwidTable, @@ -42,8 +41,8 @@ import { isContinentsVariableId } from "./GrapherConstants" export const legacyToOwidTableAndDimensions = ( json: MultipleOwidVariableDataDimensionsMap, - grapherConfig: Partial -): { dimensions: OwidChartDimensionInterface[]; table: OwidTable } => { + dimensions: OwidChartDimensionInterface[] +): OwidTable => { // Entity meta map const entityMeta = [...json.values()].flatMap( @@ -53,8 +52,6 @@ export const legacyToOwidTableAndDimensions = ( entityMeta.map((entity) => [entity.id.toString(), entity]) ) - const dimensions = grapherConfig.dimensions || [] - // Base column defs, shared by all variable tables const baseColumnDefs: Map = new Map() @@ -62,25 +59,9 @@ export const legacyToOwidTableAndDimensions = ( baseColumnDefs.set(def.slug, def) }) - const entityColorColumnSlug = grapherConfig.selectedEntityColors - ? OwidTableSlugs.entityColor - : undefined - if (entityColorColumnSlug) { - baseColumnDefs.set(entityColorColumnSlug, { - slug: entityColorColumnSlug, - type: ColumnTypeNames.Color, - name: entityColorColumnSlug, - }) - } - // We need to create a column for each unique [variable, targetTime] pair. So there can be // multiple columns for a single variable. - const newDimensions = dimensions.map((dimension) => ({ - ...dimension, - slug: dimension.targetYear - ? `${dimension.variableId}-${dimension.targetYear}` - : `${dimension.variableId}`, - })) + const newDimensions = computeActualDimensions(dimensions) const dimensionColumns = uniqBy(newDimensions, (dim) => dim.slug) const variableTablesToJoinByYear: OwidTable[] = [] @@ -174,18 +155,6 @@ export const legacyToOwidTableAndDimensions = ( ) columnDefs.set(annotationColumnDef.slug, annotationColumnDef) } - - if (entityColorColumnSlug) { - columnStore[entityColorColumnSlug] = entityIds.map((entityId) => { - // see comment above about entityMetaById[id] - const entityName = entityMetaById[entityId]?.name - const selectedEntityColors = grapherConfig.selectedEntityColors - return entityName && selectedEntityColors - ? selectedEntityColors[entityName] - : undefined - }) - } - // Build the tables let variableTable = new OwidTable( @@ -198,12 +167,7 @@ export const legacyToOwidTableAndDimensions = ( // We do this by dropping the column. We interpolate before which adds an originalTime // column which can be used to recover the time. const targetTime = dimension?.targetYear - const chartType = grapherConfig.chartTypes?.[0] - if ( - (chartType === GRAPHER_CHART_TYPES.ScatterPlot || - chartType === GRAPHER_CHART_TYPES.Marimekko) && - isNumber(targetTime) - ) { + if (isNumber(targetTime)) { variableTable = variableTable // interpolateColumnWithTolerance() won't handle injecting times beyond the current // allTimes. So if targetYear is 2018, and we have data up to 2017, the @@ -359,7 +323,34 @@ export const legacyToOwidTableAndDimensions = ( } } - return { dimensions: newDimensions, table: joinedVariablesTable } + return joinedVariablesTable +} + +export const addSelectedEntityColorsToTable = ( + table: OwidTable, + selectedEntityColors: { [entityName: string]: string | undefined } +): OwidTable => { + const entityColorColumnSlug = OwidTableSlugs.entityColor + + const valueFn = (entityId: EntityId | undefined) => { + if (!entityId) return ErrorValueTypes.UndefinedButShouldBeString + const entityName = table.entityIdToNameMap.get(entityId) + return entityName && selectedEntityColors + ? (selectedEntityColors[entityName] ?? + ErrorValueTypes.UndefinedButShouldBeString) + : ErrorValueTypes.UndefinedButShouldBeString + } + + const values = table.rows.map((row) => valueFn(row.entityId)) + + return table.appendColumns([ + { + slug: entityColorColumnSlug, + name: entityColorColumnSlug, + type: ColumnTypeNames.Color, + values: values, + }, + ]) } const fullJoinTables = ( @@ -747,6 +738,17 @@ const annotationsToMap = (annotations: string): Map => { return entityAnnotationsMap } +export function computeActualDimensions( + dimensions: OwidChartDimensionInterface[] +): OwidChartDimensionInterface[] { + return dimensions.map((dimension) => ({ + ...dimension, + slug: dimension.targetYear + ? `${dimension.variableId}-${dimension.targetYear}` + : `${dimension.variableId}`, + })) +} + /** * Loads a single variable into an OwidTable. */