From dcdaf21c4fb95ff6bfb3caa322949fa1e43c6c2b Mon Sep 17 00:00:00 2001 From: Sophia Mersmann Date: Tue, 17 Dec 2024 18:36:13 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20(scatter/marimekko)=20apply=20co?= =?UTF-8?q?lor/size=20tolerance=20early?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../grapher/src/core/Grapher.tsx | 27 ++++- .../scatterCharts/ScatterPlotChart.test.ts | 111 +++++++++++++----- .../src/scatterCharts/ScatterPlotChart.tsx | 24 +--- .../src/stackedCharts/MarimekkoChart.tsx | 14 +-- 4 files changed, 111 insertions(+), 65 deletions(-) diff --git a/packages/@ourworldindata/grapher/src/core/Grapher.tsx b/packages/@ourworldindata/grapher/src/core/Grapher.tsx index ccb3353b48b..d56ed150f07 100644 --- a/packages/@ourworldindata/grapher/src/core/Grapher.tsx +++ b/packages/@ourworldindata/grapher/src/core/Grapher.tsx @@ -829,9 +829,34 @@ export class Grapher return table } + @computed get tableAfterColorAndSizeToleranceApplication(): OwidTable { + let table = this.inputTable + + if (this.isScatter && this.sizeColumnSlug) { + const tolerance = + table.get(this.sizeColumnSlug)?.display?.tolerance ?? Infinity + table = table.interpolateColumnWithTolerance( + this.sizeColumnSlug, + tolerance + ) + } + + if ((this.isScatter || this.isMarimekko) && this.colorColumnSlug) { + const tolerance = + table.get(this.colorColumnSlug)?.display?.tolerance ?? Infinity + table = table.interpolateColumnWithTolerance( + this.colorColumnSlug, + tolerance + ) + } + + 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 @computed get tableAfterAuthorTimelineFilter(): OwidTable { - const table = this.inputTable + const table = this.tableAfterColorAndSizeToleranceApplication + if ( this.timelineMinTime === undefined && this.timelineMaxTime === undefined diff --git a/packages/@ourworldindata/grapher/src/scatterCharts/ScatterPlotChart.test.ts b/packages/@ourworldindata/grapher/src/scatterCharts/ScatterPlotChart.test.ts index b30d9d4cc8a..04c95cb6b9b 100755 --- a/packages/@ourworldindata/grapher/src/scatterCharts/ScatterPlotChart.test.ts +++ b/packages/@ourworldindata/grapher/src/scatterCharts/ScatterPlotChart.test.ts @@ -22,10 +22,12 @@ import { ColumnTypeNames, OwidTableSlugs, Color, + GRAPHER_CHART_TYPES, } from "@ourworldindata/types" import { ContinentColors } from "../color/CustomSchemes" import { sortBy, uniq, uniqBy } from "@ourworldindata/utils" import { ScatterPointsWithLabels } from "./ScatterPointsWithLabels" +import { Grapher } from "../core/Grapher" it("can create a new chart", () => { const manager: ScatterPlotManager = { @@ -139,14 +141,16 @@ describe("interpolation defaults", () => { }, ] ) - const manager: ScatterPlotManager = { - xColumnSlug: "x", - yColumnSlug: "y", - colorColumnSlug: "color", - sizeColumnSlug: "size", + + const grapher = new Grapher({ table, - } - const chart = new ScatterPlotChart({ manager }) + chartTypes: [GRAPHER_CHART_TYPES.ScatterPlot], + xSlug: "x", + ySlugs: "y", + colorSlug: "color", + sizeSlug: "size", + }) + const chart = grapher.chartInstance as ScatterPlotChart it("color defaults to infinity tolerance if none specified", () => { expect( @@ -195,15 +199,15 @@ describe("basic scatterplot", () => { ] ) - const manager: ScatterPlotManager = { - xColumnSlug: "x", - yColumnSlug: "y", - colorColumnSlug: "color", - sizeColumnSlug: "size", + const grapher = new Grapher({ + chartTypes: [GRAPHER_CHART_TYPES.ScatterPlot], + xSlug: "x", + ySlugs: "y", + colorSlug: "color", + sizeSlug: "size", table, - } - - const chart = new ScatterPlotChart({ manager }) + }) + const chart = grapher.chartInstance as ScatterPlotChart it("removes error values from X and Y", () => { expect(chart.transformedTable.numRows).toEqual(2) @@ -425,16 +429,16 @@ describe("entity exclusion", () => { ] ) - const manager: ScatterPlotManager = { - xColumnSlug: "x", - yColumnSlug: "y", - colorColumnSlug: "color", - sizeColumnSlug: "size", + const grapher = new Grapher({ + chartTypes: [GRAPHER_CHART_TYPES.ScatterPlot], + xSlug: "x", + ySlugs: "y", + colorSlug: "color", + sizeSlug: "size", matchingEntitiesOnly: true, table, - } - - const chart = new ScatterPlotChart({ manager }) + }) + const chart = grapher.chartInstance as ScatterPlotChart it("excludes entities without color when matchingEntitiesOnly is enabled", () => { expect(chart.allPoints.length).toEqual(1) @@ -815,15 +819,15 @@ describe("x/y tolerance", () => { ] ) - const manager: ScatterPlotManager = { - xColumnSlug: "x", - yColumnSlug: "y", - colorColumnSlug: "color", - sizeColumnSlug: "size", + const grapher = new Grapher({ + chartTypes: [GRAPHER_CHART_TYPES.ScatterPlot], + xSlug: "x", + ySlugs: "y", + colorSlug: "color", + sizeSlug: "size", table, - } - - const chart = new ScatterPlotChart({ manager }) + }) + const chart = grapher.chartInstance as ScatterPlotChart const transformedTable = chart.transformedTable @@ -1030,3 +1034,48 @@ describe("correct bubble sizes", () => { expect(sortedRenderSeries[1].fontSize).toEqual(10.5) }) }) + +it("applies color tolerance before applying the author timeline filter", () => { + const table = new OwidTable( + [ + [ + "entityId", + "entityName", + "entityCode", + "year", + "x", + "y", + "color", + "size", + ], + [1, "UK", "", -1000, 1, 1, null, null], + [1, "UK", "", 1000, 1, 1, null, 100], + [1, "UK", "", 2020, 1, 1, null, null], + [1, "UK", "", 2023, null, null, "Europe", null], + ], + [ + { slug: "x", type: ColumnTypeNames.Numeric }, + { slug: "y", type: ColumnTypeNames.Numeric }, + { slug: "color", type: ColumnTypeNames.String }, + { + slug: "size", + type: ColumnTypeNames.Numeric, + }, + ] + ) + + const grapher = new Grapher({ + table, + chartTypes: [GRAPHER_CHART_TYPES.ScatterPlot], + xSlug: "x", + ySlugs: "y", + colorSlug: "color", + sizeSlug: "size", + timelineMaxTime: 2020, + }) + const chart = grapher.chartInstance as ScatterPlotChart + + expect( + uniq(chart.transformedTable.get("color").valuesIncludingErrorValues) + ).toEqual(["Europe"]) +}) diff --git a/packages/@ourworldindata/grapher/src/scatterCharts/ScatterPlotChart.tsx b/packages/@ourworldindata/grapher/src/scatterCharts/ScatterPlotChart.tsx index bb7743b90aa..9f65ef78db9 100644 --- a/packages/@ourworldindata/grapher/src/scatterCharts/ScatterPlotChart.tsx +++ b/packages/@ourworldindata/grapher/src/scatterCharts/ScatterPlotChart.tsx @@ -171,28 +171,8 @@ export class ScatterPlotChart if (this.yScaleType === ScaleType.log && this.yColumnSlug) table = table.replaceNonPositiveCellsForLogScale([this.yColumnSlug]) - if (this.sizeColumnSlug) { - const tolerance = - table.get(this.sizeColumnSlug)?.display?.tolerance ?? Infinity - table = table.interpolateColumnWithTolerance( - this.sizeColumnSlug, - tolerance - ) - } - - if (this.colorColumnSlug) { - const tolerance = - table.get(this.colorColumnSlug)?.display?.tolerance ?? Infinity - table = table.interpolateColumnWithTolerance( - this.colorColumnSlug, - tolerance - ) - if (this.manager.matchingEntitiesOnly) { - table = table.dropRowsWithErrorValuesForColumn( - this.colorColumnSlug - ) - } - } + if (this.colorColumnSlug && this.manager.matchingEntitiesOnly) + table = table.dropRowsWithErrorValuesForColumn(this.colorColumnSlug) // We want to "chop off" any rows outside the time domain for X and Y to avoid creating // leading and trailing timeline times that don't really exist in the dataset. diff --git a/packages/@ourworldindata/grapher/src/stackedCharts/MarimekkoChart.tsx b/packages/@ourworldindata/grapher/src/stackedCharts/MarimekkoChart.tsx index 617c87dc3b3..c95f9cc3c72 100644 --- a/packages/@ourworldindata/grapher/src/stackedCharts/MarimekkoChart.tsx +++ b/packages/@ourworldindata/grapher/src/stackedCharts/MarimekkoChart.tsx @@ -318,17 +318,9 @@ export class MarimekkoChart if (xColumnSlug) table = table.interpolateColumnWithTolerance(xColumnSlug) - if (colorColumnSlug) { - const tolerance = - table.get(colorColumnSlug)?.display?.tolerance ?? Infinity - table = table.interpolateColumnWithTolerance( - colorColumnSlug, - tolerance - ) - if (manager.matchingEntitiesOnly) { - table = table.dropRowsWithErrorValuesForColumn(colorColumnSlug) - } - } + if (colorColumnSlug && manager.matchingEntitiesOnly) + table = table.dropRowsWithErrorValuesForColumn(colorColumnSlug) + if (!manager.showNoDataArea) table = table.dropRowsWithErrorValuesForAllColumns(yColumnSlugs)