From c902bef1c23dbcab80f624bd5e1f0ce505274794 Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Tue, 10 Oct 2023 11:26:59 +0000 Subject: [PATCH 1/3] enhance(data-table): hide entities not displayed in a scatter plot --- .../grapher/src/chart/ChartInterface.ts | 1 + .../@ourworldindata/grapher/src/core/Grapher.tsx | 9 +++++++++ .../grapher/src/dataTable/DataTable.tsx | 12 +++++++++--- .../src/scatterCharts/ScatterPlotChart.tsx | 16 ++++++++++++++++ 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/packages/@ourworldindata/grapher/src/chart/ChartInterface.ts b/packages/@ourworldindata/grapher/src/chart/ChartInterface.ts index b99f3d1b578..3633c46bd00 100644 --- a/packages/@ourworldindata/grapher/src/chart/ChartInterface.ts +++ b/packages/@ourworldindata/grapher/src/chart/ChartInterface.ts @@ -31,6 +31,7 @@ export interface ChartInterface { // Todo: should all charts additionally have a placedSeries: ChartPlacedSeries[] getter? transformTable: ChartTableTransformer + transformTableForDisplay?: 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 529b9f86da1..382543c6311 100644 --- a/packages/@ourworldindata/grapher/src/core/Grapher.tsx +++ b/packages/@ourworldindata/grapher/src/core/Grapher.tsx @@ -658,6 +658,15 @@ export class Grapher return this.xAxis.toObject() } + // table that is used for display in the table tab + @computed get tableForDisplay(): OwidTable { + const table = this.table + if (!this.isReady || !this.isOnTableTab) return table + return this.chartInstance.transformTableForDisplay + ? this.chartInstance.transformTableForDisplay(table) + : table + } + @computed get tableForSelection(): OwidTable { // This table specifies which entities can be selected in the charts EntitySelectorModal. // It should contain all entities that can be selected, and none more. diff --git a/packages/@ourworldindata/grapher/src/dataTable/DataTable.tsx b/packages/@ourworldindata/grapher/src/dataTable/DataTable.tsx index 4971a2c4a9b..7506e99e53f 100644 --- a/packages/@ourworldindata/grapher/src/dataTable/DataTable.tsx +++ b/packages/@ourworldindata/grapher/src/dataTable/DataTable.tsx @@ -75,7 +75,8 @@ const inverseSortOrder = (order: SortOrder): SortOrder => order === SortOrder.asc ? SortOrder.desc : SortOrder.asc export interface DataTableManager { - table: OwidTable + table: OwidTable // not used here, but required in type `ChartManager` + tableForDisplay: OwidTable entityType?: string endTime?: Time startTime?: Time @@ -129,7 +130,7 @@ export class DataTable extends React.Component<{ } @computed get table(): OwidTable { - let table = this.manager.table + let table = this.manager.tableForDisplay if (this.manager.showSelectionOnlyInDataTable) { table = table.filterByEntityNames( this.selectionArray.selectedEntityNames @@ -139,7 +140,12 @@ export class DataTable extends React.Component<{ } @computed get manager(): DataTableManager { - return this.props.manager ?? { table: BlankOwidTable() } + return ( + this.props.manager ?? { + table: BlankOwidTable(), + tableForDisplay: BlankOwidTable(), + } + ) } @computed private get entityType(): string { diff --git a/packages/@ourworldindata/grapher/src/scatterCharts/ScatterPlotChart.tsx b/packages/@ourworldindata/grapher/src/scatterCharts/ScatterPlotChart.tsx index 87e2e6323b1..835a5b55036 100644 --- a/packages/@ourworldindata/grapher/src/scatterCharts/ScatterPlotChart.tsx +++ b/packages/@ourworldindata/grapher/src/scatterCharts/ScatterPlotChart.tsx @@ -265,6 +265,22 @@ export class ScatterPlotChart return table } + transformTableForDisplay(table: OwidTable): OwidTable { + // Drop any rows which have non-number values for X or Y. + table = table + .columnFilter( + this.xColumnSlug, + isNumber, + "Drop rows with non-number values in X column" + ) + .columnFilter( + this.yColumnSlug, + isNumber, + "Drop rows with non-number values in Y column" + ) + return table + } + @computed get inputTable(): OwidTable { return this.manager.table } From 371d45e1fc32c5fabd5b0624ba6de6907b6e72da Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Mon, 16 Oct 2023 11:18:59 +0000 Subject: [PATCH 2/3] enhance(data-table): remove manually excluded entities --- .../src/scatterCharts/ScatterPlotChart.tsx | 50 +++++++++-------- .../src/stackedCharts/MarimekkoChart.tsx | 56 +++++++++++-------- 2 files changed, 61 insertions(+), 45 deletions(-) diff --git a/packages/@ourworldindata/grapher/src/scatterCharts/ScatterPlotChart.tsx b/packages/@ourworldindata/grapher/src/scatterCharts/ScatterPlotChart.tsx index 835a5b55036..4e948453691 100644 --- a/packages/@ourworldindata/grapher/src/scatterCharts/ScatterPlotChart.tsx +++ b/packages/@ourworldindata/grapher/src/scatterCharts/ScatterPlotChart.tsx @@ -120,6 +120,27 @@ export class ScatterPlotChart series: ScatterSeries }>() + private filterManuallySelectedEntities(table: OwidTable): OwidTable { + const { includedEntities, excludedEntities } = this.manager + const excludedEntityIdsSet = new Set(excludedEntities) + const includedEntityIdsSet = new Set(includedEntities) + const excludeEntitiesFilter = (entityId: any): boolean => + !excludedEntityIdsSet.has(entityId as number) + const includedEntitiesFilter = (entityId: any): boolean => + includedEntityIdsSet.size > 0 + ? includedEntityIdsSet.has(entityId as number) + : true + const filterFn = (entityId: any): boolean => + excludeEntitiesFilter(entityId) && includedEntitiesFilter(entityId) + const excludedList = excludedEntities ? excludedEntities.join(", ") : "" + const includedList = includedEntities ? includedEntities.join(", ") : "" + return table.columnFilter( + OwidTableSlugs.entityId, + filterFn, + `Excluded entity ids specified by author: ${excludedList} - Included entity ids specified by author: ${includedList}` + ) + } + transformTable(table: OwidTable): OwidTable { const { backgroundSeriesLimit, @@ -138,28 +159,7 @@ export class ScatterPlotChart } if (excludedEntities || includedEntities) { - const excludedEntityIdsSet = new Set(excludedEntities) - const includedEntityIdsSet = new Set(includedEntities) - const excludeEntitiesFilter = (entityId: any): boolean => - !excludedEntityIdsSet.has(entityId as number) - const includedEntitiesFilter = (entityId: any): boolean => - includedEntityIdsSet.size > 0 - ? includedEntityIdsSet.has(entityId as number) - : true - const filterFn = (entityId: any): boolean => - excludeEntitiesFilter(entityId) && - includedEntitiesFilter(entityId) - const excludedList = excludedEntities - ? excludedEntities.join(", ") - : "" - const includedList = includedEntities - ? includedEntities.join(", ") - : "" - table = table.columnFilter( - OwidTableSlugs.entityId, - filterFn, - `Excluded entity ids specified by author: ${excludedList} - Included entity ids specified by author: ${includedList}` - ) + table = this.filterManuallySelectedEntities(table) } // Allow authors to limit the # of background entities to get better perf and clearer charts. @@ -266,6 +266,12 @@ export class ScatterPlotChart } transformTableForDisplay(table: OwidTable): OwidTable { + const { includedEntities, excludedEntities } = this.manager + + if (excludedEntities || includedEntities) { + table = this.filterManuallySelectedEntities(table) + } + // Drop any rows which have non-number values for X or Y. table = table .columnFilter( diff --git a/packages/@ourworldindata/grapher/src/stackedCharts/MarimekkoChart.tsx b/packages/@ourworldindata/grapher/src/stackedCharts/MarimekkoChart.tsx index 3d50f8a6781..eb83bb0f5e5 100644 --- a/packages/@ourworldindata/grapher/src/stackedCharts/MarimekkoChart.tsx +++ b/packages/@ourworldindata/grapher/src/stackedCharts/MarimekkoChart.tsx @@ -255,35 +255,35 @@ export class MarimekkoChart entityName: string }>() + private filterManuallySelectedEntities(table: OwidTable): OwidTable { + const { excludedEntities, includedEntities } = this.manager + const excludedEntityIdsSet = new Set(excludedEntities) + const includedEntityIdsSet = new Set(includedEntities) + const excludeEntitiesFilter = (entityId: any): boolean => + !excludedEntityIdsSet.has(entityId as number) + const includedEntitiesFilter = (entityId: any): boolean => + includedEntityIdsSet.size > 0 + ? includedEntityIdsSet.has(entityId as number) + : true + const filterFn = (entityId: any): boolean => + excludeEntitiesFilter(entityId) && includedEntitiesFilter(entityId) + const excludedList = excludedEntities ? excludedEntities.join(", ") : "" + const includedList = includedEntities ? includedEntities.join(", ") : "" + return table.columnFilter( + OwidTableSlugs.entityId, + filterFn, + `Excluded entity ids specified by author: ${excludedList} - Included entity ids specified by author: ${includedList}` + ) + } + transformTable(table: OwidTable): OwidTable { const { excludedEntities, includedEntities } = this.manager const { yColumnSlugs, manager, colorColumnSlug, xColumnSlug } = this if (!this.yColumnSlugs.length) return table if (excludedEntities || includedEntities) { - const excludedEntityIdsSet = new Set(excludedEntities) - const includedEntityIdsSet = new Set(includedEntities) - const excludeEntitiesFilter = (entityId: any): boolean => - !excludedEntityIdsSet.has(entityId as number) - const includedEntitiesFilter = (entityId: any): boolean => - includedEntityIdsSet.size > 0 - ? includedEntityIdsSet.has(entityId as number) - : true - const filterFn = (entityId: any): boolean => - excludeEntitiesFilter(entityId) && - includedEntitiesFilter(entityId) - const excludedList = excludedEntities - ? excludedEntities.join(", ") - : "" - const includedList = includedEntities - ? includedEntities.join(", ") - : "" - table = table.columnFilter( - OwidTableSlugs.entityId, - filterFn, - `Excluded entity ids specified by author: ${excludedList} - Included entity ids specified by author: ${includedList}` - ) - } else table = table + this.filterManuallySelectedEntities(table) + } // TODO: remove this filter once we don't have mixed type columns in datasets table = table.replaceNonNumericCellsWithErrorValues(yColumnSlugs) @@ -329,6 +329,16 @@ export class MarimekkoChart return table } + transformTableForDisplay(table: OwidTable): OwidTable { + const { includedEntities, excludedEntities } = this.manager + + if (excludedEntities || includedEntities) { + table = this.filterManuallySelectedEntities(table) + } + + return table + } + @computed get inputTable(): OwidTable { return this.manager.table } From 666ce4f5a742f2ddfa2f0a5d22b4c1b0a4222bd1 Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Mon, 16 Oct 2023 12:02:22 +0000 Subject: [PATCH 3/3] fix(marimekko): actually honor filtering --- .../grapher/src/stackedCharts/MarimekkoChart.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@ourworldindata/grapher/src/stackedCharts/MarimekkoChart.tsx b/packages/@ourworldindata/grapher/src/stackedCharts/MarimekkoChart.tsx index eb83bb0f5e5..06c9b8aa932 100644 --- a/packages/@ourworldindata/grapher/src/stackedCharts/MarimekkoChart.tsx +++ b/packages/@ourworldindata/grapher/src/stackedCharts/MarimekkoChart.tsx @@ -282,7 +282,7 @@ export class MarimekkoChart if (!this.yColumnSlugs.length) return table if (excludedEntities || includedEntities) { - this.filterManuallySelectedEntities(table) + table = this.filterManuallySelectedEntities(table) } // TODO: remove this filter once we don't have mixed type columns in datasets