From 7e331e130f5a76dd13f84c3afba549a93f16b282 Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Fri, 13 Oct 2023 12:43:08 +0000 Subject: [PATCH 1/7] fix(grapher): remove entities that are not plotted from the entity selector --- .../src/barCharts/DiscreteBarChart.tsx | 18 +++++++++++++++ .../grapher/src/chart/ChartInterface.ts | 1 + .../grapher/src/core/Grapher.tsx | 23 +++++++++++-------- .../grapher/src/lineCharts/LineChart.tsx | 16 +++++++++++++ .../stackedCharts/AbstractStackedChart.tsx | 23 +++++++++++++++++++ .../stackedCharts/StackedDiscreteBarChart.tsx | 22 ++++++++++++++++++ 6 files changed, 94 insertions(+), 9 deletions(-) diff --git a/packages/@ourworldindata/grapher/src/barCharts/DiscreteBarChart.tsx b/packages/@ourworldindata/grapher/src/barCharts/DiscreteBarChart.tsx index f89d9d7568a..cd70d6bb355 100644 --- a/packages/@ourworldindata/grapher/src/barCharts/DiscreteBarChart.tsx +++ b/packages/@ourworldindata/grapher/src/barCharts/DiscreteBarChart.tsx @@ -128,6 +128,24 @@ export class DiscreteBarChart return table } + transformTableForSelection(table: OwidTable): OwidTable { + table = table + .replaceNonNumericCellsWithErrorValues(this.yColumnSlugs) + .dropRowsWithErrorValuesForAllColumns(this.yColumnSlugs) + + if (this.missingDataStrategy === MissingDataStrategy.hide) { + const groupedByEntity = table.groupBy("entityName").map((t) => { + if (t.hasAnyColumnNoValidValue(this.yColumnSlugs)) { + t = t.dropAllRows() + } + return t + }) + table = groupedByEntity[0].concat(groupedByEntity.slice(1)) + } + + return table + } + @computed get inputTable(): OwidTable { return this.manager.table } diff --git a/packages/@ourworldindata/grapher/src/chart/ChartInterface.ts b/packages/@ourworldindata/grapher/src/chart/ChartInterface.ts index 3633c46bd00..c7363ec394d 100644 --- a/packages/@ourworldindata/grapher/src/chart/ChartInterface.ts +++ b/packages/@ourworldindata/grapher/src/chart/ChartInterface.ts @@ -32,6 +32,7 @@ export interface ChartInterface { transformTable: ChartTableTransformer transformTableForDisplay?: ChartTableTransformer + transformTableForSelection?: ChartTableTransformer yAxis?: HorizontalAxis | VerticalAxis xAxis?: HorizontalAxis | VerticalAxis diff --git a/packages/@ourworldindata/grapher/src/core/Grapher.tsx b/packages/@ourworldindata/grapher/src/core/Grapher.tsx index 2a234de47c5..4ac6ebb6a8d 100644 --- a/packages/@ourworldindata/grapher/src/core/Grapher.tsx +++ b/packages/@ourworldindata/grapher/src/core/Grapher.tsx @@ -675,16 +675,21 @@ export class Grapher // Depending on the chart type, the criteria for being able to select an entity are // different; e.g. for scatterplots, the entity needs to (1) not be excluded and // (2) needs to have data for the x and y dimension. + let table = + this.isScatter || this.isSlopeChart + ? this.tableAfterAuthorTimelineAndActiveChartTransform + : this.inputTable + + if (!this.isReady) return table + + // Some chart types (e.g. stacked area charts) choose not to show an entity + // with incomplete data. Such chart types define a custom transform function + // to ensure that the entity selector only offers entities that are actually plotted. + if (this.chartInstance.transformTableForSelection) { + table = this.chartInstance.transformTableForSelection(table) + } - if (this.isScatter || this.isSlopeChart) - // for scatter and slope charts, the `transformTable()` call takes care of removing - // all entities that cannot be selected - return this.tableAfterAuthorTimelineAndActiveChartTransform - - // for other chart types, the `transformTable()` call would sometimes remove too many - // entities, and we want to use the inputTable instead (which should have exactly the - // entities where data is available) - return this.inputTable + return table } // If an author sets a timeline filter run it early in the pipeline so to the charts it's as if the filtered times do not exist diff --git a/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx b/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx index 756a80f8228..92b61700f68 100644 --- a/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx +++ b/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx @@ -300,6 +300,22 @@ export class LineChart return table } + transformTableForSelection(table: OwidTable): OwidTable { + table = table.replaceNonNumericCellsWithErrorValues(this.yColumnSlugs) + + if (this.missingDataStrategy === MissingDataStrategy.hide) { + const groupedByEntity = table.groupBy("entityName").map((t) => { + if (t.hasAnyColumnNoValidValue(this.yColumnSlugs)) { + t = t.dropAllRows() + } + return t + }) + table = groupedByEntity[0].concat(groupedByEntity.slice(1)) + } + + return table + } + @computed private get missingDataStrategy(): MissingDataStrategy { return this.manager.missingDataStrategy || MissingDataStrategy.auto } diff --git a/packages/@ourworldindata/grapher/src/stackedCharts/AbstractStackedChart.tsx b/packages/@ourworldindata/grapher/src/stackedCharts/AbstractStackedChart.tsx index 0da08a59baa..cfc48a56257 100644 --- a/packages/@ourworldindata/grapher/src/stackedCharts/AbstractStackedChart.tsx +++ b/packages/@ourworldindata/grapher/src/stackedCharts/AbstractStackedChart.tsx @@ -84,6 +84,29 @@ export class AbstractStackedChart return table } + transformTableForSelection(table: OwidTable): OwidTable { + table = table + .replaceNonNumericCellsWithErrorValues(this.yColumnSlugs) + .dropRowsWithErrorValuesForAllColumns(this.yColumnSlugs) + + if (this.shouldRunLinearInterpolation) { + this.yColumnSlugs.forEach((slug) => { + table = table.interpolateColumnLinearly(slug) + }) + } + + if (this.missingDataStrategy !== MissingDataStrategy.show) { + const groupedByEntity = table + .groupBy("entityName") + .map((t: OwidTable) => + t.dropRowsWithErrorValuesForAnyColumn(this.yColumnSlugs) + ) + table = groupedByEntity[0].concat(groupedByEntity.slice(1)) + } + + return table + } + @computed private get missingDataStrategy(): MissingDataStrategy { return this.manager.missingDataStrategy || MissingDataStrategy.auto } diff --git a/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx b/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx index 341b6b8c61c..e12e27c59de 100644 --- a/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx +++ b/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx @@ -132,6 +132,28 @@ export class StackedDiscreteBarChart return table } + transformTableForSelection(table: OwidTable): OwidTable { + table = table + .replaceNonNumericCellsWithErrorValues(this.yColumnSlugs) + .dropRowsWithErrorValuesForAllColumns(this.yColumnSlugs) + + this.yColumnSlugs.forEach((slug) => { + table = table.interpolateColumnWithTolerance(slug) + }) + + if (this.missingDataStrategy === MissingDataStrategy.hide) { + const groupedByEntity = table.groupBy("entityName").map((t) => { + if (t.hasAnyColumnNoValidValue(this.yColumnSlugs)) { + t = t.dropAllRows() + } + return t + }) + table = groupedByEntity[0].concat(groupedByEntity.slice(1)) + } + + return table + } + @computed get sortConfig(): SortConfig { return this.manager.sortConfig ?? {} } From dc5939f9b0347624aabd3f750ae8d7640c59d3b2 Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Tue, 17 Oct 2023 16:40:22 +0000 Subject: [PATCH 2/7] enhance(grapher): only run transforms for tableForSelection if necessary --- .../src/barCharts/DiscreteBarChart.tsx | 10 ++++++---- .../grapher/src/lineCharts/LineChart.tsx | 8 ++++++-- .../stackedCharts/AbstractStackedChart.tsx | 20 ++++++++++--------- .../stackedCharts/StackedDiscreteBarChart.tsx | 16 ++++++++------- 4 files changed, 32 insertions(+), 22 deletions(-) diff --git a/packages/@ourworldindata/grapher/src/barCharts/DiscreteBarChart.tsx b/packages/@ourworldindata/grapher/src/barCharts/DiscreteBarChart.tsx index cd70d6bb355..333f187cad9 100644 --- a/packages/@ourworldindata/grapher/src/barCharts/DiscreteBarChart.tsx +++ b/packages/@ourworldindata/grapher/src/barCharts/DiscreteBarChart.tsx @@ -129,11 +129,13 @@ export class DiscreteBarChart } transformTableForSelection(table: OwidTable): OwidTable { - table = table - .replaceNonNumericCellsWithErrorValues(this.yColumnSlugs) - .dropRowsWithErrorValuesForAllColumns(this.yColumnSlugs) - + // 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) + .dropRowsWithErrorValuesForAllColumns(this.yColumnSlugs) + const groupedByEntity = table.groupBy("entityName").map((t) => { if (t.hasAnyColumnNoValidValue(this.yColumnSlugs)) { t = t.dropAllRows() diff --git a/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx b/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx index 92b61700f68..1ac1822e4d5 100644 --- a/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx +++ b/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx @@ -301,9 +301,13 @@ export class LineChart } transformTableForSelection(table: OwidTable): OwidTable { - table = table.replaceNonNumericCellsWithErrorValues(this.yColumnSlugs) - + // 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 + ) + const groupedByEntity = table.groupBy("entityName").map((t) => { if (t.hasAnyColumnNoValidValue(this.yColumnSlugs)) { t = t.dropAllRows() diff --git a/packages/@ourworldindata/grapher/src/stackedCharts/AbstractStackedChart.tsx b/packages/@ourworldindata/grapher/src/stackedCharts/AbstractStackedChart.tsx index cfc48a56257..dc82f986683 100644 --- a/packages/@ourworldindata/grapher/src/stackedCharts/AbstractStackedChart.tsx +++ b/packages/@ourworldindata/grapher/src/stackedCharts/AbstractStackedChart.tsx @@ -85,17 +85,19 @@ export class AbstractStackedChart } transformTableForSelection(table: OwidTable): OwidTable { - table = table - .replaceNonNumericCellsWithErrorValues(this.yColumnSlugs) - .dropRowsWithErrorValuesForAllColumns(this.yColumnSlugs) + // if entities with partial data are not plotted, + // make sure they don't show up in the entity selector + if (this.missingDataStrategy !== MissingDataStrategy.show) { + table = table + .replaceNonNumericCellsWithErrorValues(this.yColumnSlugs) + .dropRowsWithErrorValuesForAllColumns(this.yColumnSlugs) - if (this.shouldRunLinearInterpolation) { - this.yColumnSlugs.forEach((slug) => { - table = table.interpolateColumnLinearly(slug) - }) - } + if (this.shouldRunLinearInterpolation) { + this.yColumnSlugs.forEach((slug) => { + table = table.interpolateColumnLinearly(slug) + }) + } - if (this.missingDataStrategy !== MissingDataStrategy.show) { const groupedByEntity = table .groupBy("entityName") .map((t: OwidTable) => diff --git a/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx b/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx index e12e27c59de..c399cb81dd3 100644 --- a/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx +++ b/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx @@ -133,15 +133,17 @@ export class StackedDiscreteBarChart } transformTableForSelection(table: OwidTable): OwidTable { - table = table - .replaceNonNumericCellsWithErrorValues(this.yColumnSlugs) - .dropRowsWithErrorValuesForAllColumns(this.yColumnSlugs) + // 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) + .dropRowsWithErrorValuesForAllColumns(this.yColumnSlugs) - this.yColumnSlugs.forEach((slug) => { - table = table.interpolateColumnWithTolerance(slug) - }) + this.yColumnSlugs.forEach((slug) => { + table = table.interpolateColumnWithTolerance(slug) + }) - if (this.missingDataStrategy === MissingDataStrategy.hide) { const groupedByEntity = table.groupBy("entityName").map((t) => { if (t.hasAnyColumnNoValidValue(this.yColumnSlugs)) { t = t.dropAllRows() From 5e0c5c8f0386b73bd29b23783f1d3e6c51004800 Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Wed, 25 Oct 2023 09:41:18 +0200 Subject: [PATCH 3/7] chore(grapher): remove custom tableForSelection for (stacked) discrete bar charts --- .../src/barCharts/DiscreteBarChart.tsx | 20 ---------------- .../stackedCharts/StackedDiscreteBarChart.tsx | 24 ------------------- 2 files changed, 44 deletions(-) diff --git a/packages/@ourworldindata/grapher/src/barCharts/DiscreteBarChart.tsx b/packages/@ourworldindata/grapher/src/barCharts/DiscreteBarChart.tsx index 333f187cad9..f89d9d7568a 100644 --- a/packages/@ourworldindata/grapher/src/barCharts/DiscreteBarChart.tsx +++ b/packages/@ourworldindata/grapher/src/barCharts/DiscreteBarChart.tsx @@ -128,26 +128,6 @@ export class DiscreteBarChart return table } - transformTableForSelection(table: OwidTable): OwidTable { - // 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) - .dropRowsWithErrorValuesForAllColumns(this.yColumnSlugs) - - const groupedByEntity = table.groupBy("entityName").map((t) => { - if (t.hasAnyColumnNoValidValue(this.yColumnSlugs)) { - t = t.dropAllRows() - } - return t - }) - table = groupedByEntity[0].concat(groupedByEntity.slice(1)) - } - - return table - } - @computed get inputTable(): OwidTable { return this.manager.table } diff --git a/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx b/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx index c399cb81dd3..341b6b8c61c 100644 --- a/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx +++ b/packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx @@ -132,30 +132,6 @@ export class StackedDiscreteBarChart return table } - transformTableForSelection(table: OwidTable): OwidTable { - // 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) - .dropRowsWithErrorValuesForAllColumns(this.yColumnSlugs) - - this.yColumnSlugs.forEach((slug) => { - table = table.interpolateColumnWithTolerance(slug) - }) - - const groupedByEntity = table.groupBy("entityName").map((t) => { - if (t.hasAnyColumnNoValidValue(this.yColumnSlugs)) { - t = t.dropAllRows() - } - return t - }) - table = groupedByEntity[0].concat(groupedByEntity.slice(1)) - } - - return table - } - @computed get sortConfig(): SortConfig { return this.manager.sortConfig ?? {} } From 21a86d1855e2b9ee0a7b0472602f3a0dce48f256 Mon Sep 17 00:00:00 2001 From: Sophia Mersmann Date: Thu, 26 Oct 2023 09:20:50 +0200 Subject: [PATCH 4/7] refactor(line-chart): simplify transformTableForSelection Co-authored-by: Marcel Gerber --- .../@ourworldindata/grapher/src/lineCharts/LineChart.tsx | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx b/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx index 1ac1822e4d5..9945554ecb0 100644 --- a/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx +++ b/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx @@ -308,12 +308,9 @@ export class LineChart this.yColumnSlugs ) - const groupedByEntity = table.groupBy("entityName").map((t) => { - if (t.hasAnyColumnNoValidValue(this.yColumnSlugs)) { - t = t.dropAllRows() - } - return t - }) + const groupedByEntity = table.groupBy("entityName").filter((t) => + !t.hasAnyColumnNoValidValue(this.yColumnSlugs) + ) table = groupedByEntity[0].concat(groupedByEntity.slice(1)) } From d3b65d48f9c12571ed4eb8415431a3d422139a02 Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Thu, 26 Oct 2023 07:23:45 +0000 Subject: [PATCH 5/7] =?UTF-8?q?=F0=9F=A4=96=20style:=20prettify=20code?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../@ourworldindata/grapher/src/lineCharts/LineChart.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx b/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx index 9945554ecb0..2e88976e680 100644 --- a/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx +++ b/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx @@ -308,9 +308,9 @@ export class LineChart this.yColumnSlugs ) - const groupedByEntity = table.groupBy("entityName").filter((t) => - !t.hasAnyColumnNoValidValue(this.yColumnSlugs) - ) + const groupedByEntity = table + .groupBy("entityName") + .filter((t) => !t.hasAnyColumnNoValidValue(this.yColumnSlugs)) table = groupedByEntity[0].concat(groupedByEntity.slice(1)) } From 75147aa660ae51d459aa547db17e759693989fc5 Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Thu, 26 Oct 2023 07:45:39 +0000 Subject: [PATCH 6/7] refactor: guard against an empty list of tables --- packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx b/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx index 2e88976e680..5bd03433464 100644 --- a/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx +++ b/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx @@ -66,6 +66,7 @@ import { CoreValueType, isNotErrorValue, EntityName, + BlankOwidTable, } from "@ourworldindata/core-table" import { autoDetectSeriesStrategy, @@ -311,6 +312,9 @@ export class LineChart const groupedByEntity = table .groupBy("entityName") .filter((t) => !t.hasAnyColumnNoValidValue(this.yColumnSlugs)) + + if (groupedByEntity.length === 0) return BlankOwidTable() + table = groupedByEntity[0].concat(groupedByEntity.slice(1)) } From 3e99975f68d305dde450871b4c3107eba776a333 Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Mon, 30 Oct 2023 09:58:31 +0000 Subject: [PATCH 7/7] fix: guard against an empty list of tables --- .../grapher/src/stackedCharts/AbstractStackedChart.tsx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/@ourworldindata/grapher/src/stackedCharts/AbstractStackedChart.tsx b/packages/@ourworldindata/grapher/src/stackedCharts/AbstractStackedChart.tsx index dc82f986683..284da9838e3 100644 --- a/packages/@ourworldindata/grapher/src/stackedCharts/AbstractStackedChart.tsx +++ b/packages/@ourworldindata/grapher/src/stackedCharts/AbstractStackedChart.tsx @@ -24,7 +24,11 @@ import { StackedRawSeries, StackedSeries, } from "./StackedConstants" -import { OwidTable, CoreColumn } from "@ourworldindata/core-table" +import { + OwidTable, + CoreColumn, + BlankOwidTable, +} from "@ourworldindata/core-table" import { autoDetectSeriesStrategy, autoDetectYColumnSlugs, @@ -103,6 +107,9 @@ export class AbstractStackedChart .map((t: OwidTable) => t.dropRowsWithErrorValuesForAnyColumn(this.yColumnSlugs) ) + + if (groupedByEntity.length === 0) return BlankOwidTable() + table = groupedByEntity[0].concat(groupedByEntity.slice(1)) }