Stabilize plot paths (#2811)

* do not drop previously collected plot paths

* do not display plots without datapoints

Author: mattseddon

extension/src/plots/model/collect.ts

@@ -594,6 +594,44 @@ const fillTemplate = (
   ) as TopLevelSpec
 }
 
+const collectTemplatePlot = (
+  acc: TemplatePlotEntry[],
+  selectedRevisions: string[],
+  path: string,
+  template: string,
+  revisionData: RevisionData,
+  size: number,
+  revisionColors: ColorScale | undefined,
+  multiSourceEncoding: MultiSourceEncoding
+) => {
+  const isMultiView = isMultiViewPlot(JSON.parse(template))
+  const multiSourceEncodingUpdate = multiSourceEncoding[path] || {}
+  const { datapoints, revisions } = transformRevisionData(
+    path,
+    selectedRevisions,
+    revisionData,
+    isMultiView,
+    multiSourceEncodingUpdate
+  )
+
+  if (datapoints.length === 0) {
+    return
+  }
+
+  const content = extendVegaSpec(fillTemplate(template, datapoints), size, {
+    ...multiSourceEncodingUpdate,
+    color: revisionColors
+  })
+
+  acc.push({
+    content,
+    id: path,
+    multiView: isMultiViewPlot(content),
+    revisions,
+    type: PlotsType.VEGA
+  })
+}
+
 const collectTemplateGroup = (
   paths: string[],
   selectedRevisions: string[],
@@ -607,30 +645,20 @@ const collectTemplateGroup = (
   for (const path of paths) {
     const template = templates[path]
 
-    if (template) {
-      const isMultiView = isMultiViewPlot(JSON.parse(template))
-      const multiSourceEncodingUpdate = multiSourceEncoding[path] || {}
-      const { datapoints, revisions } = transformRevisionData(
-        path,
-        selectedRevisions,
-        revisionData,
-        isMultiView,
-        multiSourceEncodingUpdate
-      )
-
-      const content = extendVegaSpec(fillTemplate(template, datapoints), size, {
-        ...multiSourceEncodingUpdate,
-        color: revisionColors
-      })
-
-      acc.push({
-        content,
-        id: path,
-        multiView: isMultiViewPlot(content),
-        revisions,
-        type: PlotsType.VEGA
-      })
+    if (!template) {
+      continue
     }
+
+    collectTemplatePlot(
+      acc,
+      selectedRevisions,
+      path,
+      template,
+      revisionData,
+      size,
+      revisionColors,
+      multiSourceEncoding
+    )
   }
   return acc
 }

extension/src/plots/paths/collect.test.ts

@@ -1,18 +1,20 @@
 import { join } from 'path'
 import { VisualizationSpec } from 'react-vega'
+import isEqual from 'lodash.isequal'
 import {
   collectEncodingElements,
   collectPaths,
   collectTemplateOrder,
-  EncodingType
+  EncodingType,
+  PathType
 } from './collect'
 import { TemplatePlotGroup, PlotsType } from '../webview/contract'
 import plotsDiffFixture from '../../test/fixtures/plotsDiff/output'
 import { Shape, StrokeDash } from '../multiSource/constants'
 
 describe('collectPath', () => {
   it('should return the expected data from the test fixture', () => {
-    expect(collectPaths(plotsDiffFixture)).toStrictEqual([
+    expect(collectPaths([], plotsDiffFixture)).toStrictEqual([
       {
         hasChildren: false,
         label: 'acc.png',
@@ -65,6 +67,21 @@ describe('collectPath', () => {
     ])
   })
 
+  it('should not drop already collected paths', () => {
+    const mockPath = 'completely:madeup:path'
+    const mockPlotPath = {
+      hasChildren: false,
+      label: mockPath,
+      parentPath: undefined,
+      path: mockPath,
+      type: new Set([PathType.TEMPLATE_SINGLE])
+    }
+
+    const paths = collectPaths([mockPlotPath], plotsDiffFixture)
+
+    expect(paths.some(plotPath => isEqual(plotPath, mockPlotPath))).toBeTruthy()
+  })
+
   it('should handle more complex paths', () => {
     const mockPlotsDiff = {
       [join('logs', 'scalars', 'acc.tsv')]: [
@@ -99,7 +116,7 @@ describe('collectPath', () => {
       ]
     }
 
-    expect(collectPaths(mockPlotsDiff)).toStrictEqual([
+    expect(collectPaths([], mockPlotsDiff)).toStrictEqual([
       {
         hasChildren: false,
         label: 'acc.tsv',

extension/src/plots/paths/collect.ts

@@ -26,6 +26,22 @@ export type PlotPath = {
   hasChildren: boolean
 }
 
+type PathAccumulator = {
+  plotPaths: PlotPath[]
+  included: Set<string>
+}
+
+const createPathAccumulator = (existingPaths: PlotPath[]): PathAccumulator => {
+  const acc: PathAccumulator = { included: new Set<string>(), plotPaths: [] }
+
+  for (const existing of existingPaths) {
+    acc.included.add(existing.path)
+    acc.plotPaths.push(existing)
+  }
+
+  return acc
+}
+
 const collectType = (plots: Plot[]) => {
   const type = new Set<PathType>()
 
@@ -60,11 +76,6 @@ const getType = (
   return collectType(plots)
 }
 
-type PathAccumulator = {
-  plotPaths: PlotPath[]
-  included: Set<string>
-}
-
 const collectPath = (
   acc: PathAccumulator,
   data: PlotsOutput,
@@ -95,8 +106,11 @@ const collectPath = (
   acc.included.add(path)
 }
 
-export const collectPaths = (data: PlotsOutput): PlotPath[] => {
-  const acc: PathAccumulator = { included: new Set<string>(), plotPaths: [] }
+export const collectPaths = (
+  existingPaths: PlotPath[],
+  data: PlotsOutput
+): PlotPath[] => {
+  const acc = createPathAccumulator(existingPaths)
 
   const paths = Object.keys(data)
 

extension/src/plots/paths/model.ts

@@ -27,7 +27,7 @@ export class PathsModel extends PathSelectionModel<PlotPath> {
   }
 
   public transformAndSet(data: PlotsOutput) {
-    const paths = collectPaths(data)
+    const paths = collectPaths(this.data, data)
 
     this.setNewStatuses(paths)