✨ (slope) refactor

packages/@ourworldindata/core-table/src/CoreTableColumns.ts

@@ -305,12 +305,22 @@ export abstract class AbstractCoreColumn<JS_TYPE extends PrimitiveType> {
     @imemo get displayName(): string {
         return (
             this.display?.name ??
-            this.def.presentation?.titlePublic ?? // this is a bit of an unusual fallback - if display.name is not given, titlePublic is the next best thing before name
+            // this is a bit of an unusual fallback - if display.name is not given, titlePublic is the next best thing before name
+            this.def.presentation?.titlePublic ??
             this.name ??
             ""
         )
     }
 
+    @imemo get nonEmptyDisplayName(): string {
+        return (
+            this.display?.name ||
+            // this is a bit of an unusual fallback - if display.name is not given, titlePublic is the next best thing before name
+            this.def.presentation?.titlePublic ||
+            this.nonEmptyName
+        )
+    }
+
     @imemo get titlePublicOrDisplayName(): IndicatorTitleWithFragments {
         return this.def.presentation?.titlePublic
             ? {

packages/@ourworldindata/core-table/src/OwidTable.ts

@@ -124,9 +124,8 @@ export class OwidTable extends CoreTable<OwidRow, OwidColumnDef> {
         return min(this.allTimes) as Time
     }
 
-    // TODO: remove undefined?
-    @imemo get maxTime(): Time | undefined {
-        return max(this.allTimes)
+    @imemo get maxTime(): Time {
+        return max(this.allTimes) as Time
     }
 
     @imemo private get allTimes(): Time[] {

packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx

@@ -102,6 +102,13 @@ import {
     HorizontalColorLegendManager,
     HorizontalNumericColorLegend,
 } from "../horizontalColorLegend/HorizontalColorLegends"
+import {
+    AnnotationsMap,
+    getAnnotationsForSeries,
+    getAnnotationsMap,
+    getColorKey,
+    getSeriesName,
+} from "./lineChartHelpers"
 
 const LINE_CHART_CLASS_NAME = "LineChart"
 
@@ -1148,24 +1155,8 @@ export class LineChart
 
     // End of color legend props
 
-    // todo: for now just works with 1 y column
-    @computed private get annotationsMap(): Map<
-        PrimitiveType,
-        Set<PrimitiveType>
-    > {
-        return this.inputTable
-            .getAnnotationColumnForColumn(this.yColumnSlugs[0])
-            ?.getUniqueValuesGroupedBy(this.inputTable.entityNameSlug)
-    }
-
-    getAnnotationsForSeries(seriesName: SeriesName): string | undefined {
-        const annotationsMap = this.annotationsMap
-        const annos = annotationsMap?.get(seriesName)
-        return annos
-            ? Array.from(annos.values())
-                  .filter((anno) => anno)
-                  .join(" & ")
-            : undefined
+    @computed private get annotationsMap(): AnnotationsMap | undefined {
+        return getAnnotationsMap(this.inputTable, this.yColumnSlugs[0])
     }
 
     @computed private get colorScheme(): ColorScheme {
@@ -1196,39 +1187,6 @@ export class LineChart
         })
     }
 
-    private getSeriesName(
-        entityName: EntityName,
-        columnName: string,
-        entityCount: number
-    ): SeriesName {
-        if (this.seriesStrategy === SeriesStrategy.entity) {
-            return entityName
-        }
-        if (entityCount > 1 || this.manager.canSelectMultipleEntities) {
-            return `${entityName} - ${columnName}`
-        } else {
-            return columnName
-        }
-    }
-
-    private getColorKey(
-        entityName: EntityName,
-        columnName: string,
-        entityCount: number
-    ): SeriesName {
-        if (this.seriesStrategy === SeriesStrategy.entity) {
-            return entityName
-        }
-        // If only one entity is plotted, we want to use the column colors.
-        // Unlike in `getSeriesName`, we don't care whether the user can select
-        // multiple entities, only whether more than one is plotted.
-        if (entityCount > 1) {
-            return `${entityName} - ${columnName}`
-        } else {
-            return columnName
-        }
-    }
-
     // cache value for performance
     @computed private get rowIndicesByEntityName(): Map<string, number[]> {
         return this.transformedTable.rowIndex([
@@ -1237,14 +1195,20 @@ export class LineChart
     }
 
     private constructSingleSeries(
-        entityName: string,
-        col: CoreColumn
+        entityName: EntityName,
+        column: CoreColumn
     ): LineChartSeries {
-        const { hasColorScale, transformedTable, colorColumn } = this
+        const {
+            manager: { canSelectMultipleEntities = false },
+            transformedTable: { availableEntityNames },
+            seriesStrategy,
+            hasColorScale,
+            colorColumn,
+        } = this
 
         // Construct the points
-        const timeValues = col.originalTimeColumn.valuesIncludingErrorValues
-        const values = col.valuesIncludingErrorValues
+        const timeValues = column.originalTimeColumn.valuesIncludingErrorValues
+        const values = column.valuesIncludingErrorValues
         const colorValues = colorColumn.valuesIncludingErrorValues
         // If Y and Color are the same column, we need to get rid of any duplicate rows.
         // Duplicates occur because Y doesn't have tolerance applied, but Color does.
@@ -1269,26 +1233,34 @@ export class LineChart
         })
 
         // Construct series properties
-        const totalEntityCount = transformedTable.availableEntityNames.length
-        const seriesName = this.getSeriesName(
+        const columnName = column.nonEmptyDisplayName
+        const seriesName = getSeriesName({
             entityName,
-            col.displayName,
-            totalEntityCount
-        )
+            columnName,
+            seriesStrategy,
+            availableEntityNames,
+            canSelectMultipleEntities,
+        })
+
         let seriesColor: Color
         if (hasColorScale) {
             const colorValue = last(points)?.colorValue
             seriesColor = this.getColorScaleColor(colorValue)
         } else {
             seriesColor = this.categoricalColorAssigner.assign(
-                this.getColorKey(entityName, col.displayName, totalEntityCount)
+                getColorKey({
+                    entityName,
+                    columnName,
+                    seriesStrategy,
+                    availableEntityNames,
+                })
             )
         }
 
         return {
             points,
             seriesName,
-            isProjection: col.isProjection,
+            isProjection: column.isProjection,
             color: seriesColor,
         }
     }
@@ -1350,7 +1322,10 @@ export class LineChart
                 seriesName,
                 // E.g. https://ourworldindata.org/grapher/size-poverty-gap-world
                 label: !this.manager.showLegend ? "" : `${seriesName}`,
-                annotation: this.getAnnotationsForSeries(seriesName),
+                annotation: getAnnotationsForSeries(
+                    this.annotationsMap,
+                    seriesName
+                ),
                 yValue: lastValue,
             }
         })

packages/@ourworldindata/grapher/src/lineCharts/lineChartHelpers.ts

@@ -1,4 +1,13 @@
-import { EntityName, SeriesName, SeriesStrategy } from "@ourworldindata/types"
+import { OwidTable } from "@ourworldindata/core-table"
+import {
+    ColumnSlug,
+    EntityName,
+    PrimitiveType,
+    SeriesName,
+    SeriesStrategy,
+} from "@ourworldindata/types"
+
+export type AnnotationsMap = Map<PrimitiveType, Set<PrimitiveType>>
 
 export function getSeriesName({
     entityName,
@@ -44,3 +53,23 @@ export function getColorKey({
         ? `${entityName} - ${columnName}`
         : columnName
 }
+
+export function getAnnotationsMap(
+    table: OwidTable,
+    slug: ColumnSlug
+): AnnotationsMap | undefined {
+    return table
+        .getAnnotationColumnForColumn(slug)
+        ?.getUniqueValuesGroupedBy(table.entityNameSlug)
+}
+
+export function getAnnotationsForSeries(
+    annotationsMap: AnnotationsMap | undefined,
+    seriesName: SeriesName
+): string | undefined {
+    const annotations = annotationsMap?.get(seriesName)
+    if (!annotations) return undefined
+    return Array.from(annotations.values())
+        .filter((anno) => anno)
+        .join(" & ")
+}

packages/@ourworldindata/grapher/src/scatterCharts/NoDataSection.tsx

@@ -6,19 +6,19 @@ import {
 } from "../core/GrapherConstants"
 
 export function NoDataSection({
-    entityNames,
+    seriesNames,
     bounds,
     baseFontSize = 16,
 }: {
-    entityNames: string[]
+    seriesNames: string[]
     bounds: Bounds
     baseFontSize?: number
 }): React.ReactElement {
     {
-        const displayedEntities = entityNames.slice(0, 5)
-        const numRemainingEntities = Math.max(
+        const displayedNames = seriesNames.slice(0, 5)
+        const remaining = Math.max(
             0,
-            entityNames.length - displayedEntities.length
+            seriesNames.length - displayedNames.length
         )
 
         return (
@@ -40,7 +40,7 @@ export function NoDataSection({
                     No data
                 </div>
                 <ul>
-                    {displayedEntities.map((entityName) => (
+                    {displayedNames.map((entityName) => (
                         <li
                             key={entityName}
                             style={{
@@ -53,14 +53,8 @@ export function NoDataSection({
                         </li>
                     ))}
                 </ul>
-                {numRemainingEntities > 0 && (
-                    <div>
-                        &{" "}
-                        {numRemainingEntities === 1
-                            ? "one"
-                            : numRemainingEntities}{" "}
-                        more
-                    </div>
+                {remaining > 0 && (
+                    <div>& {remaining === 1 ? "one" : remaining} more</div>
                 )}
             </foreignObject>
         )

packages/@ourworldindata/grapher/src/scatterCharts/ScatterPlotChart.tsx

@@ -871,7 +871,7 @@ export class ScatterPlotChart
                         {!this.manager.isStatic &&
                             separatorLine(noDataSectionBounds.top)}
                         <NoDataSection
-                            entityNames={this.selectedEntitiesWithoutData}
+                            seriesNames={this.selectedEntitiesWithoutData}
                             bounds={noDataSectionBounds}
                             baseFontSize={this.fontSize}
                         />

packages/@ourworldindata/grapher/src/slopeCharts/SlopeChart.test.ts

@@ -48,10 +48,8 @@ it("filters non-numeric values", () => {
     const chart = new SlopeChart({ manager })
     expect(chart.series.length).toEqual(1)
     expect(
-        chart.series.every((series) =>
-            series.values.every(
-                (value) => isNumber(value.x) && isNumber(value.y)
-            )
+        chart.series.every(
+            (series) => isNumber(series.startValue) && isNumber(series.endValue)
         )
     ).toBeTruthy()
 })