✨ (line chart) set y-axis min default to 0

owid · Oct 31, 2024 · 635477e · 635477e
1 parent a3b1e38
commit 635477e
Show file tree

Hide file tree

Showing 9 changed files with 136 additions and 20 deletions.
diff --git a/adminSiteClient/EditorCustomizeTab.tsx b/adminSiteClient/EditorCustomizeTab.tsx
@@ -577,9 +577,9 @@ export class EditorCustomizeTab<
                                         }
                                         resetButton={{
                                             onClick: () =>
-                                                (yAxisConfig.min = undefined),
+                                                (yAxisConfig.min = Infinity),
                                             disabled:
-                                                yAxisConfig.min === undefined,
+                                                yAxisConfig.min === Infinity,
                                         }}
                                         allowDecimal
                                         allowNegative
@@ -592,9 +592,9 @@ export class EditorCustomizeTab<
                                         }
                                         resetButton={{
                                             onClick: () =>
-                                                (yAxisConfig.max = undefined),
+                                                (yAxisConfig.max = -Infinity),
                                             disabled:
-                                                yAxisConfig.max === undefined,
+                                                yAxisConfig.max === -Infinity,
                                         }}
                                         allowDecimal
                                         allowNegative

diff --git a/db/migration/1729763649580-SetYAxisMinDefaultToZero.ts b/db/migration/1729763649580-SetYAxisMinDefaultToZero.ts
@@ -0,0 +1,95 @@
+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> {
+        // charts with inheritance disabled:
+        // set yAxis.min explicitly to "auto" for line 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
+                -- using JSON_MERGE_PATCH instead of JSON_SET in case yAxis doesn't exist
+                cc.patch = JSON_MERGE_PATCH(cc.patch, '{"yAxis":{"min":"auto"}}')
+            WHERE
+                cc.full ->> '$.type' = 'LineChart'
+                AND c.isInheritanceEnabled IS FALSE
+                AND cc.patch ->> '$.yAxis.min' IS NULL
+        `)
+
+        // charts with inheritance enabled:
+        // set yAxis.min explicitly to "auto" for line charts
+        // that used to rely on "auto" being the default.
+        // this is the case for charts where neither the patch config
+        // of the chart itself nor the indicator config have yAxis.min set
+        await queryRunner.query(`
+            -- sql
+            UPDATE chart_configs cc
+            JOIN charts c ON cc.id = c.configId
+            -- keep charts without parent indicator
+            LEFT JOIN charts_x_parents cxp ON cxp.chartId = c.id
+            LEFT JOIN variables v ON v.id = cxp.variableId
+            LEFT JOIN chart_configs cc_admin ON cc_admin.id = v.grapherConfigIdAdmin
+            SET
+                -- using JSON_MERGE_PATCH instead of JSON_SET in case yAxis doesn't exist
+                cc.patch = JSON_MERGE_PATCH(cc.patch, '{"yAxis":{"min":"auto"}}')
+            WHERE
+                cc.full ->> '$.type' = 'LineChart'
+                AND c.isInheritanceEnabled IS TRUE
+                AND cc.patch ->> '$.yAxis.min' IS NULL
+                AND (
+                    cc_admin.full IS NULL
+                    OR cc_admin.full ->> '$.yAxis.min' IS NULL
+                )
+        `)
+
+        // recompute all full configs (we need to update all charts
+        // since the defaultGrapherConfig has changed)
+        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
+             LEFT JOIN charts_x_parents cxp ON cxp.chartId = c.id
+             LEFT JOIN variables v ON v.id = cxp.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
+         `)
+        for (const chart of charts) {
+            const patchConfig = JSON.parse(chart.patchConfig)
+            const etlConfig = chart.etlConfig ? JSON.parse(chart.etlConfig) : {}
+            const adminConfig = chart.adminConfig
+                ? JSON.parse(chart.adminConfig)
+                : {}
+
+            const fullConfig = mergeGrapherConfigs(
+                defaultGrapherConfig,
+                etlConfig,
+                adminConfig,
+                patchConfig
+            )
+
+            await queryRunner.query(
+                `
+                    -- sql
+                    UPDATE chart_configs cc
+                    SET cc.full = ?
+                    WHERE cc.id = ?
+                `,
+                [JSON.stringify(fullConfig), chart.configId]
+            )
+        }
+    }
+
+    // eslint-disable-next-line @typescript-eslint/no-empty-function
+    public async down(): Promise<void> {}
+}
diff --git a/explorer/GrapherGrammar.ts b/explorer/GrapherGrammar.ts
@@ -15,14 +15,14 @@ import {
     EnumCellDef,
     Grammar,
     IntegerCellDef,
-    NumericCellDef,
     SlugDeclarationCellDef,
     SlugsDeclarationCellDef,
     StringCellDef,
     UrlCellDef,
     IndicatorIdsOrEtlPathsCellDef,
     IndicatorIdOrEtlPathCellDef,
     GrapherCellDef,
+    NumericOrAutoCellDef,
 } from "../gridLang/GridLangConstants.js"
 
 const toTerminalOptions = (keywords: string[]): CellDef[] => {
@@ -176,7 +176,7 @@ export const GrapherGrammar: Grammar<GrapherCellDef> = {
         toGrapherObject: (value) => ({ yAxis: { canChangeScaleType: value } }),
     },
     yAxisMin: {
-        ...NumericCellDef,
+        ...NumericOrAutoCellDef,
         keyword: "yAxisMin",
         description: "Set the minimum value for the yAxis",
         toGrapherObject: (value) => ({ yAxis: { min: value } }),

diff --git a/gridLang/GridLangConstants.ts b/gridLang/GridLangConstants.ts
@@ -102,6 +102,19 @@ export const NumericCellDef: CellDef = {
     parse: (value: any) => parseFloat(value),
 }
 
+export const NumericOrAutoCellDef: CellDef = {
+    keyword: "",
+    cssClass: "NumericCellDef",
+    description: "",
+    regex: /^(-?\d+\.?\d*|auto)$/,
+    requirementsDescription: `Must be a number or "auto"`,
+    valuePlaceholder: "98.6",
+    parse: (value: any) => {
+        if (value === "auto") return "auto"
+        return parseFloat(value)
+    },
+}
+
 export const IntegerCellDef: CellDef = {
     keyword: "",
     cssClass: "IntegerCellDef",

diff --git a/packages/@ourworldindata/grapher/src/axis/AxisConfig.test.ts b/packages/@ourworldindata/grapher/src/axis/AxisConfig.test.ts
@@ -3,15 +3,15 @@
 import { AxisConfig } from "../axis/AxisConfig"
 import { ScaleType } from "@ourworldindata/types"
 
-it("serializes max to 'auto' if undefined", () => {
-    const axis = new AxisConfig({ min: 1 })
+it("serializes max to 'auto' if -Infinity", () => {
+    const axis = new AxisConfig({ min: 1, max: -Infinity })
     const obj = axis.toObject()
     expect(obj.min).toBe(1)
     expect(obj.max).toBe("auto")
 })
 
-it("serializes min to 'auto' if undefined", () => {
-    const axis = new AxisConfig({ max: 0 })
+it("serializes min to 'auto' if Infinity", () => {
+    const axis = new AxisConfig({ max: 0, min: Infinity })
     const obj = axis.toObject()
     expect(obj.max).toBe(0)
     expect(obj.min).toBe("auto")

diff --git a/packages/@ourworldindata/grapher/src/axis/AxisConfig.ts b/packages/@ourworldindata/grapher/src/axis/AxisConfig.ts
@@ -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
 }
 
@@ -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 {
@@ -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
     }

diff --git a/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx b/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx
@@ -1344,6 +1344,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

diff --git a/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts b/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts
@@ -26,7 +26,6 @@ export const defaultGrapherConfig = {
     maxTime: "latest",
     yAxis: {
         removePointsOutsideDomain: false,
-        min: "auto",
         scaleType: "linear",
         max: "auto",
         canChangeScaleType: false,
@@ -66,7 +65,6 @@ export const defaultGrapherConfig = {
     },
     xAxis: {
         removePointsOutsideDomain: false,
-        min: "auto",
         scaleType: "linear",
         max: "auto",
         canChangeScaleType: false,

diff --git a/packages/@ourworldindata/grapher/src/schema/grapher-schema.005.yaml b/packages/@ourworldindata/grapher/src/schema/grapher-schema.005.yaml
@@ -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