Skip to content

Commit

Permalink
🔨 Working on getting all tests to run again, a few are still missing
Browse files Browse the repository at this point in the history
  • Loading branch information
danyx23 committed Jan 21, 2025
1 parent d844023 commit 2abf5d5
Show file tree
Hide file tree
Showing 8 changed files with 165 additions and 85 deletions.
2 changes: 1 addition & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"--watch"
],
"console": "integratedTerminal",
"runtimeExecutable": "/run/user/1000/fnm_multishells/4944_1737025638170/bin/node"
"runtimeExecutable": "/run/user/1000/fnm_multishells/6623_1737371412748/bin/node"
// "internalConsoleOptions": "neverOpen"
},
{
Expand Down
1 change: 0 additions & 1 deletion packages/@ourworldindata/explorer/src/Explorer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,6 @@ export class Explorer
if (this.grapher) {
this.grapher.grapherState.inputTable =
this.inputTableTransformer(table)
this.grapher.appendNewEntitySelectionOptions()
}
}

Expand Down
141 changes: 91 additions & 50 deletions packages/@ourworldindata/grapher/src/core/Grapher.jsdom.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ import {
OwidDistinctLinesColorScheme,
} from "../color/CustomSchemes"
import { latestGrapherConfigSchema } from "./GrapherConstants.js"
import {
legacyToOwidTableAndDimensions,

Check warning on line 40 in packages/@ourworldindata/grapher/src/core/Grapher.jsdom.test.ts

View workflow job for this annotation

GitHub Actions / eslint

'legacyToOwidTableAndDimensions' is defined but never used. Allowed unused vars must match /^_/u

Check warning on line 40 in packages/@ourworldindata/grapher/src/core/Grapher.jsdom.test.ts

View workflow job for this annotation

GitHub Actions / eslint

'legacyToOwidTableAndDimensions' is defined but never used. Allowed unused vars must match /^_/u

Check warning on line 40 in packages/@ourworldindata/grapher/src/core/Grapher.jsdom.test.ts

View workflow job for this annotation

GitHub Actions / eslint

'legacyToOwidTableAndDimensions' is defined but never used. Allowed unused vars must match /^_/u
legacyToOwidTableAndDimensionsWithMandatorySlug,
} from "./LegacyToOwidTable.js"

const TestGrapherConfig = (): {
table: OwidTable
Expand Down Expand Up @@ -149,17 +153,27 @@ const legacyConfig: Omit<LegacyGrapherInterface, "data"> &

it("can apply legacy chart dimension settings", () => {
const grapher = new GrapherState(legacyConfig)
grapher.inputTable = legacyToOwidTableAndDimensionsWithMandatorySlug(
legacyConfig.owidDataset!,
legacyConfig.dimensions!,
legacyConfig.selectedEntityColors
)
const col = grapher.yColumnsFromDimensions[0]!
expect(col.unit).toEqual(unit)
expect(col.displayName).toEqual(name)
})

it("correctly identifies changes to passed-in selection", () => {
const selection = new SelectionArray()
const selection = new SelectionArray(legacyConfig.selectedEntityNames)
const grapher = new GrapherState({
...legacyConfig,
manager: { selection },
})
grapher.inputTable = legacyToOwidTableAndDimensionsWithMandatorySlug(
legacyConfig.owidDataset!,
legacyConfig.dimensions!,
legacyConfig.selectedEntityColors
)

expect(grapher.changedParams).toEqual({})
expect(selection.selectedEntityNames).toEqual(["Iceland", "Afghanistan"])
Expand All @@ -185,6 +199,11 @@ it("can generate a url with country selection even if there is no entity code",
selectedEntityNames: [],
}
const grapher = new GrapherState(config)
grapher.inputTable = legacyToOwidTableAndDimensionsWithMandatorySlug(
config.owidDataset!,
config.dimensions!,
config.selectedEntityColors
)
expect(grapher.queryStr).toBe("")
grapher.selection.selectAll()
expect(grapher.queryStr).toContain("AFG")
Expand All @@ -197,6 +216,11 @@ it("can generate a url with country selection even if there is no entity code",
(entity) => entity.id === 15
)!.code = undefined as any
const grapher2 = new GrapherState(config2)
grapher2.inputTable = legacyToOwidTableAndDimensionsWithMandatorySlug(
config2.owidDataset!,
config2.dimensions!,
config2.selectedEntityColors
)
expect(grapher2.queryStr).toBe("")
grapher2.selection.selectAll()
expect(grapher2.queryStr).toContain("AFG")
Expand All @@ -205,6 +229,11 @@ it("can generate a url with country selection even if there is no entity code",
describe("hasTimeline", () => {
it("charts with timeline", () => {
const grapher = new GrapherState(legacyConfig)
grapher.inputTable = legacyToOwidTableAndDimensionsWithMandatorySlug(
legacyConfig.owidDataset!,
legacyConfig.dimensions!,
legacyConfig.selectedEntityColors
)
grapher.chartTypes = [GRAPHER_CHART_TYPES.LineChart]
expect(grapher.hasTimeline).toBeTruthy()
grapher.chartTypes = [GRAPHER_CHART_TYPES.SlopeChart]
Expand All @@ -219,6 +248,11 @@ describe("hasTimeline", () => {

it("map tab has timeline even if chart doesn't", () => {
const grapher = new GrapherState(legacyConfig)
grapher.inputTable = legacyToOwidTableAndDimensionsWithMandatorySlug(
legacyConfig.owidDataset!,
legacyConfig.dimensions!,
legacyConfig.selectedEntityColors
)
grapher.hideTimeline = true
grapher.chartTypes = [GRAPHER_CHART_TYPES.LineChart]
expect(grapher.hasTimeline).toBeFalsy()
Expand All @@ -229,61 +263,68 @@ describe("hasTimeline", () => {
})
})

const getGrapher = (): GrapherState =>
new GrapherState({
const getGrapher = (): GrapherState => {
const dataset = new Map([
[
142609,
{
data: {
years: [-1, 0, 1, 2],
entities: [1, 2, 1, 2],
values: [51, 52, 53, 54],
},
metadata: {
id: 142609,
display: { zeroDay: "2020-01-21", yearIsDay: true },
dimensions: {
entities: {
values: [
{
name: "United Kingdom",
code: "GBR",
id: 1,
},
{ name: "Ireland", code: "IRL", id: 2 },
],
},
years: {
values: [
{
id: -1,
},
{
id: 0,
},
{
id: 1,
},
{
id: 2,
},
],
},
},
},
},
],
])
const state = new GrapherState({
dimensions: [
{
variableId: 142609,
property: DimensionProperty.y,
},
],
owidDataset: new Map([
[
142609,
{
data: {
years: [-1, 0, 1, 2],
entities: [1, 2, 1, 2],
values: [51, 52, 53, 54],
},
metadata: {
id: 142609,
display: { zeroDay: "2020-01-21", yearIsDay: true },
dimensions: {
entities: {
values: [
{
name: "United Kingdom",
code: "GBR",
id: 1,
},
{ name: "Ireland", code: "IRL", id: 2 },
],
},
years: {
values: [
{
id: -1,
},
{
id: 0,
},
{
id: 1,
},
{
id: 2,
},
],
},
},
},
},
],
]),
minTime: -5000,
maxTime: 5000,
})
state.inputTable = legacyToOwidTableAndDimensionsWithMandatorySlug(
dataset,
state.dimensions,
{}
)
return state
}

function fromQueryParams(
params: LegacyGrapherQueryParams,
Expand Down Expand Up @@ -326,7 +367,7 @@ describe("currentTitle", () => {
)
const grapher = new GrapherState({
table,
selectedEntityNames: table.availableEntityNames,
selectedEntityNames: [...table.availableEntityNames],
dimensions: [
{
slug: SampleColumnSlugs.GDP,
Expand Down Expand Up @@ -374,7 +415,7 @@ describe("authors can use maxTime", () => {
const grapher = new GrapherState({
table,
chartTypes: [GRAPHER_CHART_TYPES.DiscreteBar],
selectedEntityNames: table.availableEntityNames,
selectedEntityNames: [...table.availableEntityNames],
maxTime: 2005,
ySlugs: "GDP",
})
Expand Down Expand Up @@ -591,7 +632,7 @@ describe("time domain tests", () => {
).replaceRandomCells(17, [SampleColumnSlugs.GDP], seed)
const grapher = new GrapherState({
table,
selectedEntityNames: table.availableEntityNames,
selectedEntityNames: [...table.availableEntityNames],
dimensions: [
{
slug: SampleColumnSlugs.GDP,
Expand Down
48 changes: 28 additions & 20 deletions packages/@ourworldindata/grapher/src/core/Grapher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,8 @@ export class GrapherState {
Url.fromQueryStr(options.queryStr ?? "").queryParams,
GRAPHER_QUERY_PARAM_KEYS
)
this.inputTable = options.table ?? BlankOwidTable(`initialGrapherTable`)
this._inputTable =
options.table ?? BlankOwidTable(`initialGrapherTable`)
this.initialOptions = options
this.selection =
this.manager?.selection ??
Expand Down Expand Up @@ -1030,7 +1031,30 @@ export class GrapherState {
return this.validChartTypes[0]
}
@observable.ref chartTab?: GrapherChartType
@observable.ref inputTable: OwidTable
@observable.ref _inputTable: OwidTable = new OwidTable()

get inputTable(): OwidTable {
return this._inputTable
}

set inputTable(table: OwidTable) {
this._inputTable = table
this.appendNewEntitySelectionOptions()

if (this.manager?.selection?.hasSelection) {
// Selection is managed externally, do nothing.
} else if (this.selection.hasSelection) {
// User has changed the selection, use theris
} else this.applyOriginalSelectionAsAuthored()
}
@action.bound appendNewEntitySelectionOptions(): void {
const { selection } = this
const currentEntities = selection.availableEntityNameSet
const missingEntities = this.availableEntities.filter(
(entity) => !currentEntities.has(entity.entityName)
)
selection.addAvailableEntityNames(missingEntities)
}
@computed get chartInstance(): ChartInterface {
// Note: when timeline handles on a LineChart are collapsed into a single handle, the
// LineChart turns into a DiscreteBar.
Expand Down Expand Up @@ -2838,7 +2862,7 @@ export class GrapherState {
}

@computed private get hasUserChangedTimeHandles(): boolean {
const authorsVersion = this.authorsVersion
const authorsVersion = this.legacyConfigAsAuthored
return (
this.minTime !== authorsVersion.minTime ||
this.maxTime !== authorsVersion.maxTime
Expand Down Expand Up @@ -3293,6 +3317,7 @@ export class Grapher extends React.Component<GrapherProps> {
}

// Exclusively used for the performance.measurement API, so that DevTools can show some context
// TODO: 2025-01-17 Daniel this is now obsolete - the selection update happens in the inputTable setter
@action.bound private _setInputTable(
json: MultipleOwidVariableDataDimensionsMap,
legacyConfig: Partial<LegacyGrapherInterface>
Expand All @@ -3311,14 +3336,6 @@ export class Grapher extends React.Component<GrapherProps> {
)

this.grapherState.inputTable = tableWithColors

this.appendNewEntitySelectionOptions()

if (this.grapherState.manager?.selection?.hasSelection) {
// Selection is managed externally, do nothing.
} else if (this.grapherState.selection.hasSelection) {
// User has changed the selection, use theris
} else this.grapherState.applyOriginalSelectionAsAuthored()
}

@action rebuildInputOwidTable(): void {
Expand All @@ -3330,15 +3347,6 @@ export class Grapher extends React.Component<GrapherProps> {
)
}

@action.bound appendNewEntitySelectionOptions(): void {
const { selection } = this.grapherState
const currentEntities = selection.availableEntityNameSet
const missingEntities = this.grapherState.availableEntities.filter(
(entity) => !currentEntities.has(entity.entityName)
)
selection.addAvailableEntityNames(missingEntities)
}

// Keeps a running cache of series colors at the Grapher level.

@bind dispose(): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,20 @@ import { GrapherProgrammaticInterface, GrapherState } from "../core/Grapher"
import { MapChart } from "../mapCharts/MapChart"
import { legacyMapGrapher } from "../mapCharts/MapChart.sample"
import { GRAPHER_CHART_TYPES } from "@ourworldindata/types"
import { legacyToOwidTableAndDimensionsWithMandatorySlug } from "./LegacyToOwidTable.js"

describe("grapher and map charts", () => {
describe("map time tolerance plus query string works with a map chart", () => {
const grapher = new GrapherState(legacyMapGrapher)
const inputTable = legacyToOwidTableAndDimensionsWithMandatorySlug(
legacyMapGrapher.owidDataset!,
legacyMapGrapher.dimensions!,
legacyMapGrapher.selectedEntityColors
)
const grapher = new GrapherState({
...legacyMapGrapher,
table: inputTable,
})

expect(grapher.mapColumnSlug).toBe("3512")
expect(grapher.inputTable.minTime).toBe(2000)
expect(grapher.inputTable.maxTime).toBe(2010)
Expand All @@ -27,6 +37,11 @@ describe("grapher and map charts", () => {

it("can change time and see more points", () => {
const manager = new GrapherState(legacyMapGrapher)
manager.inputTable = legacyToOwidTableAndDimensionsWithMandatorySlug(
legacyMapGrapher.owidDataset!,
legacyMapGrapher.dimensions!,
legacyMapGrapher.selectedEntityColors
)
const chart = new MapChart({ manager })

expect(Object.keys(chart.series).length).toEqual(1)
Expand Down
Loading

0 comments on commit 2abf5d5

Please sign in to comment.