Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 (scatter/marimekko) apply color/size tolerance early #4327

Merged
merged 1 commit into from
Dec 18, 2024
Merged

🐛 (scatter/marimekko) apply color/size tolerance early #4327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 39 additions & 1 deletion packages/@ourworldindata/grapher/src/core/Grapher.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -829,9 +829,47 @@ export class Grapher
return table
}

/**
* Input table with color and size tolerance applied.
*
* This happens _before_ applying the author's timeline filter to avoid
* accidentally dropping all color values before applying tolerance.
* This is especially important for scatter plots and Marimekko charts,
* where color and size columns are often transformed with infinite tolerance.
*
* Line and discrete bar charts also support a color dimension, but their
* tolerance transformations run in their respective transformTable functions
* since it's more efficient to run them on a table that has been filtered
* by selected entities.
*/
@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
Expand Down
111 changes: 80 additions & 31 deletions packages/@ourworldindata/grapher/src/scatterCharts/ScatterPlotChart.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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"])
})
24 changes: 2 additions & 22 deletions packages/@ourworldindata/grapher/src/scatterCharts/ScatterPlotChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
14 changes: 3 additions & 11 deletions packages/@ourworldindata/grapher/src/stackedCharts/MarimekkoChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Loading