From 46cac459529eef96de74ac5bd83555f097c64f11 Mon Sep 17 00:00:00 2001 From: Marcel Gerber Date: Mon, 29 Jul 2024 17:17:36 +0100 Subject: [PATCH 1/4] feat: implement `MissingDataStrategy` for `StackedDiscreteBar` chart --- adminSiteClient/EditorFeatures.tsx | 6 ++++- .../stackedCharts/StackedDiscreteBarChart.tsx | 24 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/adminSiteClient/EditorFeatures.tsx b/adminSiteClient/EditorFeatures.tsx index 6156e13ba59..256b8c5bb37 100644 --- a/adminSiteClient/EditorFeatures.tsx +++ b/adminSiteClient/EditorFeatures.tsx @@ -110,7 +110,11 @@ export class EditorFeatures { @computed get canSpecifyMissingDataStrategy() { if (!this.grapher.hasMultipleYColumns) return false - if (this.grapher.isStackedArea || this.grapher.isStackedBar) { + if ( + this.grapher.isStackedArea || + this.grapher.isStackedBar || + this.grapher.isStackedDiscreteBar + ) { return true } diff --git a/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx b/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx index 16d3ca41794..bfc0e2b763d 100644 --- a/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx +++ b/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx @@ -25,6 +25,7 @@ import { observer } from "mobx-react" import { ColorSchemeName, FacetStrategy, + MissingDataStrategy, SeriesName, } from "@ourworldindata/types" import { @@ -143,6 +144,12 @@ export class StackedDiscreteBarChart table = table.interpolateColumnWithTolerance(slug) }) + // If MissingDataStrategy is not explicitly set to show, drop rows (= times) where one of + // the y columns has no data + if (this.missingDataStrategy !== MissingDataStrategy.show) { + table = table.dropRowsWithErrorValuesForAnyColumn(this.yColumnSlugs) + } + if (this.manager.isRelativeMode) { table = table.toPercentageFromEachColumnForEachEntityAndTime( this.yColumnSlugs @@ -152,6 +159,23 @@ export class StackedDiscreteBarChart return table } + transformTableForSelection(table: OwidTable): OwidTable { + table = table + .replaceNonNumericCellsWithErrorValues(this.yColumnSlugs) + .replaceNegativeCellsWithErrorValues(this.yColumnSlugs) + .dropRowsWithErrorValuesForAllColumns(this.yColumnSlugs) + + if (this.missingDataStrategy !== MissingDataStrategy.show) { + table = table.dropRowsWithErrorValuesForAnyColumn(this.yColumnSlugs) + } + + return table + } + + @computed private get missingDataStrategy(): MissingDataStrategy { + return this.manager.missingDataStrategy || MissingDataStrategy.auto + } + @computed get sortConfig(): SortConfig { return this.manager.sortConfig ?? {} } From 2e42646a6d3489f4a47b5af53549da017deffd30 Mon Sep 17 00:00:00 2001 From: Marcel Gerber Date: Thu, 15 Aug 2024 11:58:05 +0100 Subject: [PATCH 2/4] enhance: make `MissingDataStrategy = show` the default, need to explicitly set it to `hide` --- .../StackedDiscreteBarChart.test.ts | 57 ++++++++++++++++++- .../stackedCharts/StackedDiscreteBarChart.tsx | 6 +- 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.test.ts b/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.test.ts index 19baa2b3dc1..4190b4205d0 100755 --- a/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.test.ts +++ b/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.test.ts @@ -1,6 +1,11 @@ #! /usr/bin/env jest -import { SortOrder, SortBy, ColumnTypeNames } from "@ourworldindata/utils" +import { + SortOrder, + SortBy, + ColumnTypeNames, + MissingDataStrategy, +} from "@ourworldindata/utils" import { OwidTable, SampleColumnSlugs, @@ -105,6 +110,10 @@ it("can display a chart with missing variable data for some entities", () => { // Check that our absolute values get properly transformed into percentages expect(chart.failMessage).toEqual("") + expect( + chart.transformTableForSelection(table).availableEntityNames + ).toEqual(["France", "Spain"]) + expect(chart.series.length).toEqual(2) expect(chart.series[0].points).toEqual([ { @@ -140,6 +149,52 @@ it("can display a chart with missing variable data for some entities", () => { ]) }) +it("can display a chart with missing variable data for some entities, while hiding missing data", () => { + const csv = `coal,gas,year,entityName + 20,,2000,France + 10,20,2000,Italy + ,14,2000,Spain` + const table = new OwidTable(csv, [ + { slug: "coal", type: ColumnTypeNames.Numeric }, + { slug: "gas", type: ColumnTypeNames.Numeric }, + { slug: "year", type: ColumnTypeNames.Year }, + ]) + + const manager: ChartManager = { + table, + selection: table.availableEntityNames, + yColumnSlugs: ["coal", "gas"], + missingDataStrategy: MissingDataStrategy.hide, + } + const chart = new StackedDiscreteBarChart({ manager }) + + // Check that our absolute values get properly transformed into percentages + expect(chart.failMessage).toEqual("") + expect( + chart.transformTableForSelection(table).availableEntityNames + ).toEqual(["Italy"]) + + expect(chart.series.length).toEqual(2) + expect(chart.series[0].points).toEqual([ + { + position: "Italy", + value: 10, + valueOffset: 0, + time: 2000, + fake: false, + }, + ]) + expect(chart.series[1].points).toEqual([ + { + position: "Italy", + value: 20, + valueOffset: 10, + time: 2000, + fake: false, + }, + ]) +}) + describe("columns as series", () => { const table = SynthesizeFruitTable({ timeRange: [2000, 2001], diff --git a/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx b/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx index bfc0e2b763d..f6fd375ec56 100644 --- a/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx +++ b/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx @@ -144,9 +144,9 @@ export class StackedDiscreteBarChart table = table.interpolateColumnWithTolerance(slug) }) - // If MissingDataStrategy is not explicitly set to show, drop rows (= times) where one of + // If MissingDataStrategy is explicitly set to hide, drop rows (= times) where one of // the y columns has no data - if (this.missingDataStrategy !== MissingDataStrategy.show) { + if (this.missingDataStrategy === MissingDataStrategy.hide) { table = table.dropRowsWithErrorValuesForAnyColumn(this.yColumnSlugs) } @@ -165,7 +165,7 @@ export class StackedDiscreteBarChart .replaceNegativeCellsWithErrorValues(this.yColumnSlugs) .dropRowsWithErrorValuesForAllColumns(this.yColumnSlugs) - if (this.missingDataStrategy !== MissingDataStrategy.show) { + if (this.missingDataStrategy === MissingDataStrategy.hide) { table = table.dropRowsWithErrorValuesForAnyColumn(this.yColumnSlugs) } From 0d16ece304e8f4a9467a0bcb53ffb8ba0d8c12a6 Mon Sep 17 00:00:00 2001 From: Marcel Gerber Date: Thu, 15 Aug 2024 12:12:07 +0100 Subject: [PATCH 3/4] enhance: with `MissingDataStrategy.auto`, drop entities with single non-error data point --- .../core-table/src/OwidTable.ts | 14 ++++++++++ .../stackedCharts/StackedDiscreteBarChart.tsx | 27 +++++++++++++------ 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/packages/@ourworldindata/core-table/src/OwidTable.ts b/packages/@ourworldindata/core-table/src/OwidTable.ts index fda142017bc..5269cdf5368 100644 --- a/packages/@ourworldindata/core-table/src/OwidTable.ts +++ b/packages/@ourworldindata/core-table/src/OwidTable.ts @@ -277,6 +277,20 @@ export class OwidTable extends CoreTable { ) } + dropRowsWithAtLeastThisManyErrorValuesForColumns( + slugs: ColumnSlug[], + minErrorValues: number + ): this { + return this.rowFilter( + (row) => + slugs.filter((slug) => !isNotErrorValue(row[slug])).length < + minErrorValues, + `Drop rows with at least ${minErrorValues} ErrorValues in every column: ${slugs.join( + ", " + )}` + ) + } + // Drop _all rows_ for an entity if there is any column that has no valid values for that entity. dropEntitiesThatHaveNoDataInSomeColumn(columnSlugs: ColumnSlug[]): this { const indexesByEntityName = this.rowIndicesByEntityName diff --git a/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx b/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx index f6fd375ec56..b1acb46152e 100644 --- a/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx +++ b/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx @@ -125,6 +125,23 @@ export class StackedDiscreteBarChart { base: React.RefObject = React.createRef() + private applyMissingDataStrategy(table: OwidTable): OwidTable { + if (this.missingDataStrategy === MissingDataStrategy.hide) { + // If MissingDataStrategy is explicitly set to hide, drop rows (= times) where one of + // the y columns has no data + return table.dropRowsWithErrorValuesForAnyColumn(this.yColumnSlugs) + } else if (this.missingDataStrategy === MissingDataStrategy.auto) { + // If MissingDataStrategy is set to auto, drop rows where there is only a single non-error value + if (this.yColumnSlugs.length > 1) { + return table.dropRowsWithAtLeastThisManyErrorValuesForColumns( + this.yColumnSlugs, + this.yColumnSlugs.length - 1 + ) + } + } + return table + } + transformTable(table: OwidTable): OwidTable { if (!this.yColumnSlugs.length) return table @@ -144,11 +161,7 @@ export class StackedDiscreteBarChart table = table.interpolateColumnWithTolerance(slug) }) - // If MissingDataStrategy is explicitly set to hide, drop rows (= times) where one of - // the y columns has no data - if (this.missingDataStrategy === MissingDataStrategy.hide) { - table = table.dropRowsWithErrorValuesForAnyColumn(this.yColumnSlugs) - } + table = this.applyMissingDataStrategy(table) if (this.manager.isRelativeMode) { table = table.toPercentageFromEachColumnForEachEntityAndTime( @@ -165,9 +178,7 @@ export class StackedDiscreteBarChart .replaceNegativeCellsWithErrorValues(this.yColumnSlugs) .dropRowsWithErrorValuesForAllColumns(this.yColumnSlugs) - if (this.missingDataStrategy === MissingDataStrategy.hide) { - table = table.dropRowsWithErrorValuesForAnyColumn(this.yColumnSlugs) - } + table = this.applyMissingDataStrategy(table) return table } From 286c2ee4470239fd7c88f8a83d1ebafb4e719522 Mon Sep 17 00:00:00 2001 From: Marcel Gerber Date: Thu, 15 Aug 2024 16:41:46 +0100 Subject: [PATCH 4/4] enhance: revert `MissingDataStrategy.auto` behavior --- .../@ourworldindata/core-table/src/OwidTable.ts | 14 -------------- .../src/stackedCharts/StackedDiscreteBarChart.tsx | 10 ++-------- 2 files changed, 2 insertions(+), 22 deletions(-) diff --git a/packages/@ourworldindata/core-table/src/OwidTable.ts b/packages/@ourworldindata/core-table/src/OwidTable.ts index 5269cdf5368..fda142017bc 100644 --- a/packages/@ourworldindata/core-table/src/OwidTable.ts +++ b/packages/@ourworldindata/core-table/src/OwidTable.ts @@ -277,20 +277,6 @@ export class OwidTable extends CoreTable { ) } - dropRowsWithAtLeastThisManyErrorValuesForColumns( - slugs: ColumnSlug[], - minErrorValues: number - ): this { - return this.rowFilter( - (row) => - slugs.filter((slug) => !isNotErrorValue(row[slug])).length < - minErrorValues, - `Drop rows with at least ${minErrorValues} ErrorValues in every column: ${slugs.join( - ", " - )}` - ) - } - // Drop _all rows_ for an entity if there is any column that has no valid values for that entity. dropEntitiesThatHaveNoDataInSomeColumn(columnSlugs: ColumnSlug[]): this { const indexesByEntityName = this.rowIndicesByEntityName diff --git a/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx b/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx index b1acb46152e..e2120575f46 100644 --- a/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx +++ b/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx @@ -130,15 +130,9 @@ export class StackedDiscreteBarChart // If MissingDataStrategy is explicitly set to hide, drop rows (= times) where one of // the y columns has no data return table.dropRowsWithErrorValuesForAnyColumn(this.yColumnSlugs) - } else if (this.missingDataStrategy === MissingDataStrategy.auto) { - // If MissingDataStrategy is set to auto, drop rows where there is only a single non-error value - if (this.yColumnSlugs.length > 1) { - return table.dropRowsWithAtLeastThisManyErrorValuesForColumns( - this.yColumnSlugs, - this.yColumnSlugs.length - 1 - ) - } } + + // Otherwise, don't apply any special treatment return table }