🎉 (slope) feature parity with line charts / TAS-708

adminSiteClient/EditorBasicTab.tsx

@@ -109,7 +109,7 @@ class DimensionSlotView<
         const { selection } = grapher
         const { availableEntityNames, availableEntityNameSet } = selection
 
-        if (grapher.isScatter || grapher.isSlopeChart || grapher.isMarimekko) {
+        if (grapher.isScatter || grapher.isMarimekko) {
             // chart types that display all entities by default shouldn't select any by default
             selection.clearSelection()
         } else if (
@@ -372,8 +372,8 @@ export class EditorBasicTab<
             grapher.stackMode = StackMode.relative
         }
 
-        // Give scatterplots and slope charts a default color dimension if they don't have one
-        if (grapher.isScatter || grapher.isSlopeChart) {
+        // Give scatterplots a default color and size dimensions
+        if (grapher.isScatter) {
             const hasColor = grapher.dimensions.find(
                 (d) => d.property === DimensionProperty.color
             )
@@ -382,10 +382,7 @@ export class EditorBasicTab<
                     variableId: CONTINENTS_INDICATOR_ID,
                     property: DimensionProperty.color,
                 })
-        }
 
-        // Give scatterplots a default size dimension if they don't have one
-        if (grapher.isScatter) {
             const hasSize = grapher.dimensions.find(
                 (d) => d.property === DimensionProperty.size
             )

adminSiteClient/EditorFeatures.tsx

@@ -62,6 +62,7 @@ export class EditorFeatures {
     @computed get hideLegend() {
         return (
             this.grapher.isLineChart ||
+            this.grapher.isSlopeChart ||
             this.grapher.isStackedArea ||
             this.grapher.isStackedDiscreteBar
         )
@@ -77,6 +78,7 @@ export class EditorFeatures {
             this.grapher.isStackedBar ||
             this.grapher.isStackedDiscreteBar ||
             this.grapher.isLineChart ||
+            this.grapher.isSlopeChart ||
             this.grapher.isScatter ||
             this.grapher.isMarimekko
         )
@@ -118,9 +120,9 @@ export class EditorFeatures {
             return true
         }
 
-        // for line charts, specifying a missing data strategy only makes sense
+        // for line and slope charts, specifying a missing data strategy only makes sense
         // if there are multiple entities
-        if (this.grapher.isLineChart) {
+        if (this.grapher.isLineChart || this.grapher.isSlopeChart) {
             return (
                 this.grapher.canChangeEntity ||
                 this.grapher.canSelectMultipleEntities
@@ -132,7 +134,7 @@ export class EditorFeatures {
 
     @computed get showChangeInPrefixToggle() {
         return (
-            this.grapher.isLineChart &&
+            (this.grapher.isLineChart || this.grapher.isSlopeChart) &&
             (this.grapher.isRelativeMode || this.grapher.canToggleRelativeMode)
         )
     }

baker/updateChartEntities.ts

@@ -106,7 +106,7 @@ const obtainAvailableEntitiesForGrapherConfig = async (
 
         // In these chart types, an unselected entity is still shown
         const chartTypeShowsUnselectedEntities =
-            grapher.isScatter || grapher.isSlopeChart || grapher.isMarimekko
+            grapher.isScatter || grapher.isMarimekko
 
         if (canChangeEntities || chartTypeShowsUnselectedEntities)
             return grapher.tableForSelection.availableEntityNames as string[]

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

@@ -325,6 +325,95 @@ export class OwidTable extends CoreTable<OwidRow, OwidColumnDef> {
         )
     }
 
+    // Drop _all rows_ for an entity if all columns have at least one invalid or missing value for that entity.
+    dropEntitiesThatHaveSomeMissingOrErrorValueInAllColumns(
+        columnSlugs: ColumnSlug[]
+    ): this {
+        const indexesByEntityName = this.rowIndicesByEntityName
+        const uniqTimes = new Set(this.allTimes)
+
+        // entity names to iterate over
+        const entityNamesToIterateOver = new Set(indexesByEntityName.keys())
+
+        // set of entities we want to keep
+        const entityNamesToKeep = new Set<string>()
+
+        // total number of entities
+        const entityCount = entityNamesToIterateOver.size
+
+        // helper function to generate operation name
+        const makeOpName = (entityNamesToKeep: Set<EntityName>): string => {
+            const entityNamesToDrop = differenceOfSets([
+                this.availableEntityNameSet,
+                entityNamesToKeep,
+            ])
+            const droppedEntitiesStr =
+                entityNamesToDrop.size > 0
+                    ? [...entityNamesToDrop].join(", ")
+                    : "(None)"
+            return `Drop entities that have some missing or error value in all column: ${columnSlugs.join(", ")}.\nDropped entities: ${droppedEntitiesStr}`
+        }
+
+        // Optimization: if there is a column that has a valid data entry for
+        // every entity and every time, we are done
+        for (let i = 0; i <= columnSlugs.length; i++) {
+            const slug = columnSlugs[i]
+            const col = this.get(slug)
+
+            if (
+                col.numValues === entityCount * uniqTimes.size &&
+                col.numErrorValues === 0
+            ) {
+                const entityNamesToKeep = new Set(indexesByEntityName.keys())
+
+                return this.columnFilter(
+                    this.entityNameSlug,
+                    (rowEntityName) =>
+                        entityNamesToKeep.has(rowEntityName as string),
+                    makeOpName(entityNamesToKeep)
+                )
+            }
+        }
+
+        for (let i = 0; i <= columnSlugs.length; i++) {
+            const slug = columnSlugs[i]
+            const col = this.get(slug)
+
+            for (const entityName of entityNamesToIterateOver) {
+                const indicesForEntityName = indexesByEntityName.get(entityName)
+                if (!indicesForEntityName)
+                    throw new Error("Unexpected: entity not found in index map")
+
+                // Optimization: If the column is missing values for the entity,
+                // we know we can't make a decision yet, so we skip this entity
+                if (indicesForEntityName.length < uniqTimes.size) continue
+
+                // Optimization: We don't care about the number of valid/error
+                // values, we just need to know if there is at least one invalid value
+                const hasSomeInvalidValueForEntityInCol =
+                    indicesForEntityName.some(
+                        (index) =>
+                            !isNotErrorValue(
+                                col.valuesIncludingErrorValues[index]
+                            )
+                    )
+
+                // Optimization: If all values are valid, we know we want to keep this entity,
+                // so we remove it from the entities to iterate over
+                if (!hasSomeInvalidValueForEntityInCol) {
+                    entityNamesToKeep.add(entityName)
+                    entityNamesToIterateOver.delete(entityName)
+                }
+            }
+        }
+
+        return this.columnFilter(
+            this.entityNameSlug,
+            (rowEntityName) => entityNamesToKeep.has(rowEntityName as string),
+            makeOpName(entityNamesToKeep)
+        )
+    }
+
     private sumsByTime(columnSlug: ColumnSlug): Map<number, number> {
         const timeValues = this.timeColumn.values
         const values = this.get(columnSlug).values as number[]

packages/@ourworldindata/grapher/src/barCharts/DiscreteBarChart.tsx

@@ -409,11 +409,14 @@ export class DiscreteBarChart
             <g id={makeIdForHumanConsumption("entity-labels")}>
                 {this.placedSeries.map((series) => {
                     return (
-                        series.label &&
-                        series.label.render(
-                            series.entityLabelX,
-                            series.barY - series.label.height / 2,
-                            { textProps: style }
+                        series.label && (
+                            <React.Fragment key={series.seriesName}>
+                                {series.label.render(
+                                    series.entityLabelX,
+                                    series.barY - series.label.height / 2,
+                                    { textProps: style }
+                                )}
+                            </React.Fragment>
                         )
                     )
                 })}
@@ -990,6 +993,7 @@ function makeProjectedDataPattern(color: string): React.ReactElement {
     const size = 7
     return (
         <pattern
+            key={color}
             id={makeProjectedDataPatternId(color)}
             patternUnits="userSpaceOnUse"
             width={size}

packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx

@@ -83,7 +83,7 @@ export interface CaptionedChartManager
     isOnMapTab?: boolean
     isOnTableTab?: boolean
     activeChartType?: GrapherChartType
-    isLineChartThatTurnedIntoDiscreteBar?: boolean
+    isLineChartThatTurnedIntoDiscreteBarActive?: boolean
     showEntitySelectionToggle?: boolean
     isExportingForSocialMedia?: boolean
 
@@ -197,7 +197,7 @@ export class CaptionedChart extends React.Component<CaptionedChartProps> {
         if (manager.isOnTableTab) return undefined
         if (manager.isOnMapTab) return GRAPHER_MAP_TYPE
         if (manager.isOnChartTab) {
-            return manager.isLineChartThatTurnedIntoDiscreteBar
+            return manager.isLineChartThatTurnedIntoDiscreteBarActive
                 ? GRAPHER_CHART_TYPES.DiscreteBar
                 : manager.activeChartType
         }

packages/@ourworldindata/grapher/src/chart/ChartUtils.tsx

@@ -1,5 +1,5 @@
 import React from "react"
-import { Box, getCountryByName } from "@ourworldindata/utils"
+import { areSetsEqual, Box, getCountryByName } from "@ourworldindata/utils"
 import {
     SeriesStrategy,
     EntityName,
@@ -15,6 +15,7 @@ import {
     GRAPHER_SIDE_PANEL_CLASS,
     GRAPHER_TIMELINE_CLASS,
     GRAPHER_SETTINGS_CLASS,
+    validChartTypeCombinations,
 } from "../core/GrapherConstants"
 
 export const autoDetectYColumnSlugs = (manager: ChartManager): string[] => {
@@ -175,3 +176,15 @@ export function mapChartTypeNameToQueryParam(
             return GRAPHER_TAB_QUERY_PARAMS.marimekko
     }
 }
+
+export function findValidChartTypeCombination(
+    chartTypes: GrapherChartType[]
+): GrapherChartType[] | undefined {
+    const chartTypeSet = new Set(chartTypes)
+    for (const validCombination of validChartTypeCombinations) {
+        const validCombinationSet = new Set(validCombination)
+        if (areSetsEqual(chartTypeSet, validCombinationSet))
+            return validCombination
+    }
+    return undefined
+}

packages/@ourworldindata/grapher/src/controls/ContentSwitchers.tsx

@@ -18,6 +18,7 @@ export interface ContentSwitchersManager {
     activeTab?: GrapherTabName
     hasMultipleChartTypes?: boolean
     setTab: (tab: GrapherTabName) => void
+    onTabChange: (oldTab: GrapherTabName, newTab: GrapherTabName) => void
     isNarrow?: boolean
     isMedium?: boolean
     isLineChartThatTurnedIntoDiscreteBar?: boolean
@@ -112,8 +113,10 @@ export class ContentSwitchers extends React.Component<{
     }
 
     @action.bound setTab(tabIndex: number): void {
+        const oldTab = this.manager.activeTab
         const newTab = this.availableTabs[tabIndex]
         this.manager.setTab(newTab)
+        this.manager.onTabChange?.(oldTab!, newTab)
     }
 
     render(): React.ReactElement {
@@ -175,9 +178,11 @@ function TabIcon({
         case GRAPHER_TAB_NAMES.WorldMap:
             return <FontAwesomeIcon icon={faEarthAmericas} />
         default:
-            const chartIcon = isLineChartThatTurnedIntoDiscreteBar
-                ? chartIcons[GRAPHER_CHART_TYPES.DiscreteBar]
-                : chartIcons[tab]
+            const chartIcon =
+                tab === GRAPHER_TAB_NAMES.LineChart &&
+                isLineChartThatTurnedIntoDiscreteBar
+                    ? chartIcons[GRAPHER_CHART_TYPES.DiscreteBar]
+                    : chartIcons[tab]
             return chartIcon
     }
 }
@@ -193,9 +198,15 @@ function makeTabLabelText(
     if (tab === GRAPHER_TAB_NAMES.WorldMap) return "Map"
     if (!options.hasMultipleChartTypes) return "Chart"
 
+    if (
+        tab === GRAPHER_TAB_NAMES.LineChart &&
+        options.isLineChartThatTurnedIntoDiscreteBar
+    )
+        return "Bar"
+
     switch (tab) {
         case GRAPHER_TAB_NAMES.LineChart:
-            return options.isLineChartThatTurnedIntoDiscreteBar ? "Bar" : "Line"
+            return "Line"
         case GRAPHER_TAB_NAMES.SlopeChart:
             return "Slope"
 

packages/@ourworldindata/grapher/src/controls/SettingsMenu.tsx

@@ -51,6 +51,7 @@ const {
     StackedDiscreteBar,
     StackedBar,
     Marimekko,
+    SlopeChart,
 } = GRAPHER_CHART_TYPES
 
 export interface SettingsMenuManager
@@ -170,6 +171,7 @@ export class SettingsMenu extends React.Component<{
             ScatterPlot,
             LineChart,
             Marimekko,
+            SlopeChart,
         ].includes(this.chartType as any)
     }
 

packages/@ourworldindata/grapher/src/controls/settings/AbsRelToggle.tsx

@@ -8,7 +8,7 @@ import {
 } from "@ourworldindata/types"
 import { LabeledSwitch } from "@ourworldindata/components"
 
-const { LineChart, ScatterPlot } = GRAPHER_CHART_TYPES
+const { LineChart, ScatterPlot, SlopeChart } = GRAPHER_CHART_TYPES
 
 export interface AbsRelToggleManager {
     stackMode?: StackMode
@@ -38,7 +38,7 @@ export class AbsRelToggle extends React.Component<{
         const { activeChartType } = this.manager
         return activeChartType === ScatterPlot
             ? "Show the percentage change per year over the the selected time range."
-            : activeChartType === LineChart
+            : activeChartType === LineChart || activeChartType === SlopeChart
               ? "Show proportional changes over time or actual values in their original units."
               : "Show values as their share of the total or as actual values in their original units."
     }