Skip to content

Commit

Permalink
✨ (line chart) set y-axis min default to 0
Browse files Browse the repository at this point in the history
  • Loading branch information
sophiamersmann committed Oct 24, 2024
1 parent f65773a commit eff8d68
Show file tree
Hide file tree
Showing 6 changed files with 145 additions and 14 deletions.
8 changes: 4 additions & 4 deletions adminSiteClient/EditorCustomizeTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -577,9 +577,9 @@ export class EditorCustomizeTab<
}
resetButton={{
onClick: () =>
(yAxisConfig.min = undefined),
(yAxisConfig.min = Infinity),
disabled:
yAxisConfig.min === undefined,
yAxisConfig.min === Infinity,
}}
allowDecimal
allowNegative
Expand All @@ -592,9 +592,9 @@ export class EditorCustomizeTab<
}
resetButton={{
onClick: () =>
(yAxisConfig.max = undefined),
(yAxisConfig.max = -Infinity),
disabled:
yAxisConfig.max === undefined,
yAxisConfig.max === -Infinity,
}}
allowDecimal
allowNegative
Expand Down
123 changes: 123 additions & 0 deletions db/migration/1729763649580-SetYAxisMinDefaultToZero.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import { defaultGrapherConfig } from "@ourworldindata/grapher"
import { mergeGrapherConfigs } from "@ourworldindata/utils"
import { MigrationInterface, QueryRunner } from "typeorm"

export class SetYAxisMinDefaultToZero1729763649580
implements MigrationInterface
{
public async up(queryRunner: QueryRunner): Promise<void> {
// when inheritance is disabled, set yAxis.min explicitly to "auto"
// for charts that used to rely on "auto" being the default
await queryRunner.query(`
-- sql
UPDATE chart_configs cc
JOIN charts c ON cc.id = c.configId
SET
cc.patch = JSON_SET(cc.patch, '$.yAxis.min', 'auto'),
cc.full = JSON_SET(cc.full, '$.yAxis.min', 'auto')
WHERE
cc.full ->> '$.type' = 'LineChart'
AND cc.patch ->> '$.yAxis.min' IS NULL
AND c.isInheritanceEnabled IS FALSE
`)

// set yAxis.min to "auto" for etl-authored configs for configs
// that used to rely on "auto" being the default
await queryRunner.query(`
-- sql
UPDATE chart_configs cc
JOIN variables v ON cc.id = v.grapherConfigIdETL
SET
cc.patch = JSON_SET(cc.patch, '$.yAxis.min', 'auto'),
cc.full = JSON_SET(cc.full, '$.yAxis.min', 'auto')
WHERE
COALESCE(cc.patch ->> '$.type', 'LineChart') = 'LineChart'
AND cc.patch ->> '$.yAxis.min' IS NULL
`)

// update admin-authored configs (we don't currently have any in use
// but included for completeness)
const indicatorConfigs = await queryRunner.query(`
-- sql
SELECT
v.id AS variableId,
cc_admin.id AS adminConfigId,
cc_admin.patch AS adminConfig,
cc_etl.patch AS etlConfig
FROM variables v
JOIN chart_configs cc_admin ON cc_admin.id = v.grapherConfigIdAdmin
JOIN chart_configs cc_etl ON cc_etl.id = v.grapherConfigIdETL
WHERE
COALESCE(cc_etl.patch ->> '$.type', 'LineChart') = 'LineChart'
AND cc_etl.patch ->> '$.yAxis.min' = 'auto'
`)

for (const indicator of indicatorConfigs) {
const fullConfig = mergeGrapherConfigs(
indicator.etlConfig ?? {},
indicator.adminConfig ?? {}
)

await queryRunner.query(
`
-- sql
UPDATE chart_configs cc
SET cc.full = ?
WHERE cc.id = ?
`,
[JSON.stringify(fullConfig), indicator.adminConfigId]
)
}

// Update the full configs of all charts that inherit from an indicator
const charts = await queryRunner.query(`
-- sql
SELECT
cc.id AS configId,
cc.patch AS patchConfig,
cc_etl.patch AS etlConfig,
cc_admin.patch AS adminConfig
FROM charts c
JOIN chart_configs cc ON cc.id = c.configId
JOIN charts_x_parents p ON p.chartId = c.id
JOIN variables v ON v.id = p.variableId
LEFT JOIN chart_configs cc_etl ON cc_etl.id = v.grapherConfigIdETL
LEFT JOIN chart_configs cc_admin ON cc_admin.id = v.grapherConfigIdAdmin
WHERE c.isInheritanceEnabled IS TRUE
`)

for (const chart of charts) {
// if neither the indicator chart nor the patch config specified a yAxis
// min value, then the chart used to rely on the default "auto" value
if (
chart.etlConfig.yAxis.min === undefined &&
chart.patchConfig.yAxis.min === undefined
) {
chart.patchConfig.yAxis.min = "auto"
}

const fullConfig = mergeGrapherConfigs(
defaultGrapherConfig,
chart.etlConfig ?? {},
chart.adminConfig ?? {},
chart.patchConfig
)

await queryRunner.query(
`
-- sql
UPDATE chart_configs cc
SET cc.full = ?
WHERE cc.id = ?
`,
[JSON.stringify(fullConfig), chart.configId]
)
}
}

public async down(): Promise<void> {
throw new Error(
"Migration SetYAxisMinDefaultToZero1729763649580 can't be reverted"
)
}
}
19 changes: 13 additions & 6 deletions packages/@ourworldindata/grapher/src/axis/AxisConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,17 @@ class AxisConfigDefaults implements AxisConfigInterface {
@observable.ref domainValues?: number[] = undefined
}

function parseAutoOrNumberFromJSON(
function parseMinFromJSON(
value: AxisMinMaxValueStr.auto | number | undefined
): number | undefined {
if (value === AxisMinMaxValueStr.auto) return undefined
if (value === AxisMinMaxValueStr.auto) return Infinity
return value
}

function parseMaxFromJSON(
value: AxisMinMaxValueStr.auto | number | undefined
): number | undefined {
if (value === AxisMinMaxValueStr.auto) return -Infinity
return value
}

Expand All @@ -67,8 +74,8 @@ export class AxisConfig
// todo: test/refactor
updateFromObject(props?: AxisConfigInterface): void {
if (props) extend(this, props)
if (props?.min) this.min = parseAutoOrNumberFromJSON(props?.min)
if (props?.max) this.max = parseAutoOrNumberFromJSON(props?.max)
if (props?.min) this.min = parseMinFromJSON(props?.min)
if (props?.max) this.max = parseMaxFromJSON(props?.max)
}

toObject(): AxisConfigInterface {
Expand Down Expand Up @@ -96,8 +103,8 @@ export class AxisConfig

deleteRuntimeAndUnchangedProps(obj, new AxisConfigDefaults())

if (obj.min === undefined) obj.min = AxisMinMaxValueStr.auto
if (obj.max === undefined) obj.max = AxisMinMaxValueStr.auto
if (obj.min === Infinity) obj.min = AxisMinMaxValueStr.auto
if (obj.max === -Infinity) obj.max = AxisMinMaxValueStr.auto

return obj
}
Expand Down
2 changes: 2 additions & 0 deletions packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1342,6 +1342,8 @@ export class LineChart
// horizontal axis to be at the bottom of the chart.
// see https://github.com/owid/owid-grapher/pull/975#issuecomment-890798547
singleValueAxisPointAlign: AxisAlign.start,
// default to 0 if not set
min: 0,
...this.manager.yAxisConfig,
},
this
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ export const defaultGrapherConfig = {
maxTime: "latest",
yAxis: {
removePointsOutsideDomain: false,
min: "auto",
scaleType: "linear",
max: "auto",
canChangeScaleType: false,
Expand Down Expand Up @@ -66,7 +65,6 @@ export const defaultGrapherConfig = {
},
xAxis: {
removePointsOutsideDomain: false,
min: "auto",
scaleType: "linear",
max: "auto",
canChangeScaleType: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -514,8 +514,9 @@ $defs:
type: string
description: Axis label
min:
description: Minimum domain value of the axis. Inferred from data if set to "auto".
default: auto
description: |
Minimum domain value of the axis. Inferred from data if set to "auto".
Usually defaults to "auto", but defaults to 0 for line charts on the y-axis.
oneOf:
- type: number
- type: string
Expand Down

0 comments on commit eff8d68

Please sign in to comment.