Remove the plot section renamer feature (#1855)

Co-authored-by: mattseddon <[email protected]>

Author: sroy3

extension/src/persistence/constants.ts

@@ -11,6 +11,5 @@ export enum PersistenceKey {
   PLOT_SECTION_COLLAPSED = 'plotSectionCollapsed:',
   PLOT_SELECTED_METRICS = 'plotSelectedMetrics:',
   PLOT_SIZES = 'plotSizes:',
-  PLOT_SECTION_NAMES = 'plotSectionNames:',
   PLOT_TEMPLATE_ORDER = 'plotTemplateOrder:'
 }

extension/src/plots/index.ts

@@ -188,7 +188,6 @@ export class Plots extends BaseRepository<TPlotsData> {
 
     return {
       plots,
-      sectionName: this.plots.getSectionName(Section.TEMPLATE_PLOTS),
       size: this.plots.getPlotSize(Section.TEMPLATE_PLOTS)
     }
   }
@@ -204,7 +203,6 @@ export class Plots extends BaseRepository<TPlotsData> {
       plots: comparison.map(({ path, revisions }) => {
         return { path, revisions: this.getRevisionsWithCorrectUrls(revisions) }
       }),
-      sectionName: this.plots.getSectionName(Section.COMPARISON_TABLE),
       size: this.plots.getPlotSize(Section.COMPARISON_TABLE)
     }
   }
@@ -248,11 +246,6 @@ export class Plots extends BaseRepository<TPlotsData> {
             )
           case MessageFromWebviewType.TOGGLE_PLOTS_SECTION:
             return this.setSectionCollapsed(message.payload)
-          case MessageFromWebviewType.RENAME_SECTION:
-            return this.setSectionName(
-              message.payload.section,
-              message.payload.name
-            )
           case MessageFromWebviewType.REORDER_PLOTS_COMPARISON:
             return this.setComparisonOrder(message.payload)
           case MessageFromWebviewType.REORDER_PLOTS_TEMPLATES:
@@ -302,15 +295,6 @@ export class Plots extends BaseRepository<TPlotsData> {
     )
   }
 
-  private setSectionName(section: Section, name: string) {
-    this.plots?.setSectionName(section, name)
-    sendTelemetryEvent(
-      EventName.VIEWS_PLOTS_RENAME_SECTION,
-      { section },
-      undefined
-    )
-  }
-
   private setComparisonOrder(order: string[]) {
     this.plots?.setComparisonOrder(order)
     this.webview?.show({

extension/src/plots/model/index.test.ts

@@ -1,7 +1,6 @@
 import { PlotsModel } from '.'
 import {
   DEFAULT_SECTION_COLLAPSED,
-  DEFAULT_SECTION_NAMES,
   DEFAULT_SECTION_SIZES,
   PlotSize,
   Section
@@ -87,49 +86,6 @@ describe('plotsModel', () => {
     )
   })
 
-  it('should change the the sectionName of a section when calling setSectionName', () => {
-    expect(model.getSectionName(Section.CHECKPOINT_PLOTS)).toStrictEqual(
-      DEFAULT_SECTION_NAMES[Section.CHECKPOINT_PLOTS]
-    )
-    expect(model.getSectionName(Section.TEMPLATE_PLOTS)).toStrictEqual(
-      DEFAULT_SECTION_NAMES[Section.TEMPLATE_PLOTS]
-    )
-
-    const newCheckpointPlotsName = 'Previously called live'
-    model.setSectionName(Section.CHECKPOINT_PLOTS, newCheckpointPlotsName)
-
-    expect(model.getSectionName(Section.CHECKPOINT_PLOTS)).toStrictEqual(
-      newCheckpointPlotsName
-    )
-    expect(model.getSectionName(Section.TEMPLATE_PLOTS)).toStrictEqual(
-      DEFAULT_SECTION_NAMES[Section.TEMPLATE_PLOTS]
-    )
-
-    const newTemplatePlotsName = 'Previously Called Static'
-    model.setSectionName(Section.TEMPLATE_PLOTS, newTemplatePlotsName)
-    expect(model.getSectionName(Section.TEMPLATE_PLOTS)).toStrictEqual(
-      newTemplatePlotsName
-    )
-  })
-
-  it('should update the persisted section names when calling setSectionName', () => {
-    const mementoUpdateSpy = jest.spyOn(memento, 'update')
-
-    const newName = 'Important Plots'
-    model.setSectionName(Section.CHECKPOINT_PLOTS, newName)
-
-    expect(mementoUpdateSpy).toHaveBeenCalledTimes(1)
-    expect(mementoUpdateSpy).toHaveBeenCalledWith(
-      PersistenceKey.PLOT_SECTION_NAMES + exampleDvcRoot,
-      {
-        [Section.CHECKPOINT_PLOTS]: newName,
-        [Section.TEMPLATE_PLOTS]: DEFAULT_SECTION_NAMES[Section.TEMPLATE_PLOTS],
-        [Section.COMPARISON_TABLE]:
-          DEFAULT_SECTION_NAMES[Section.COMPARISON_TABLE]
-      }
-    )
-  })
-
   it('should update the persisted collapsible section state when calling setSectionCollapsed', () => {
     const mementoUpdateSpy = jest.spyOn(memento, 'update')
 

extension/src/plots/model/index.ts

@@ -19,7 +19,6 @@ import {
   Revision,
   ComparisonRevisionData,
   DEFAULT_SECTION_COLLAPSED,
-  DEFAULT_SECTION_NAMES,
   DEFAULT_SECTION_SIZES,
   PlotSize,
   Section,
@@ -39,7 +38,6 @@ export class PlotsModel extends ModelWithPersistence {
 
   private plotSizes: Record<Section, PlotSize>
   private sectionCollapsed: SectionCollapsed
-  private sectionNames: Record<Section, string>
   private branchRevisions: Record<string, string> = {}
   private workspaceRunningCheckpoint: string | undefined
 
@@ -71,10 +69,6 @@ export class PlotsModel extends ModelWithPersistence {
       PersistenceKey.PLOT_SECTION_COLLAPSED,
       DEFAULT_SECTION_COLLAPSED
     )
-    this.sectionNames = this.revive(
-      PersistenceKey.PLOT_SECTION_NAMES,
-      DEFAULT_SECTION_NAMES
-    )
     this.comparisonOrder = this.revive(PersistenceKey.PLOT_COMPARISON_ORDER, [])
     this.selectedMetrics = this.revive(
       PersistenceKey.PLOT_SELECTED_METRICS,
@@ -153,7 +147,6 @@ export class PlotsModel extends ModelWithPersistence {
     return {
       colors,
       plots: this.getPlots(this.checkpointPlots, selectedExperiments),
-      sectionName: this.getSectionName(Section.CHECKPOINT_PLOTS),
       selectedMetrics: this.getSelectedMetrics(),
       size: this.getPlotSize(Section.CHECKPOINT_PLOTS)
     }
@@ -289,15 +282,6 @@ export class PlotsModel extends ModelWithPersistence {
     return this.sectionCollapsed
   }
 
-  public setSectionName(section: Section, name: string) {
-    this.sectionNames[section] = name
-    this.persist(PersistenceKey.PLOT_SECTION_NAMES, this.sectionNames)
-  }
-
-  public getSectionName(section: Section): string {
-    return this.sectionNames[section] || DEFAULT_SECTION_NAMES[section]
-  }
-
   private removeStaleData() {
     return Promise.all([
       this.removeStaleBranches(),

extension/src/plots/webview/contract.ts

@@ -16,12 +16,6 @@ export enum Section {
   COMPARISON_TABLE = 'comparison-table'
 }
 
-export const DEFAULT_SECTION_NAMES = {
-  [Section.CHECKPOINT_PLOTS]: 'Trends',
-  [Section.TEMPLATE_PLOTS]: 'Data Series',
-  [Section.COMPARISON_TABLE]: 'Images'
-}
-
 export const DEFAULT_SECTION_SIZES = {
   [Section.CHECKPOINT_PLOTS]: PlotSize.REGULAR,
   [Section.TEMPLATE_PLOTS]: PlotSize.REGULAR,
@@ -52,7 +46,6 @@ export type Revision = {
 
 export interface PlotsComparisonData {
   plots: ComparisonPlots
-  sectionName: string
   size: PlotSize
 }
 
@@ -73,7 +66,6 @@ export type CheckpointPlotsData = {
   plots: CheckpointPlotData[]
   colors: ColorScale
   size: PlotSize
-  sectionName: string
   selectedMetrics?: string[]
 }
 
@@ -121,7 +113,6 @@ export type TemplatePlotSection = {
 
 export interface TemplatePlotsData {
   plots: TemplatePlotSection[]
-  sectionName: string
   size: PlotSize
 }
 

extension/src/telemetry/constants.ts

@@ -54,7 +54,6 @@ export const EventName = Object.assign(
     VIEWS_PLOTS_FOCUS_CHANGED: 'views.plots.focusChanged',
     VIEWS_PLOTS_MANUAL_REFRESH: 'views.plots.manualRefresh',
     VIEWS_PLOTS_METRICS_SELECTED: 'views.plots.metricsSelected',
-    VIEWS_PLOTS_RENAME_SECTION: 'views.plots.sectionRenamed',
     VIEWS_PLOTS_REVISIONS_REORDERED: 'views.plots.revisionsReordered',
     VIEWS_PLOTS_SECTION_RESIZED: 'views.plots.sectionResized',
     VIEWS_PLOTS_SECTION_TOGGLE: 'views.plots.toggleSection',
@@ -207,7 +206,6 @@ export interface IEventNamePropertyMapping {
   [EventName.VIEWS_PLOTS_FOCUS_CHANGED]: WebviewFocusChangedProperties
   [EventName.VIEWS_PLOTS_MANUAL_REFRESH]: { revisions: number }
   [EventName.VIEWS_PLOTS_METRICS_SELECTED]: undefined
-  [EventName.VIEWS_PLOTS_RENAME_SECTION]: { section: Section }
   [EventName.VIEWS_PLOTS_REVISIONS_REORDERED]: undefined
   [EventName.VIEWS_PLOTS_SECTION_RESIZED]: { section: Section; size: PlotSize }
   [EventName.VIEWS_PLOTS_SECTION_TOGGLE]: Partial<SectionCollapsed>

extension/src/test/fixtures/expShow/checkpointPlots.ts

@@ -1,10 +1,5 @@
 import { copyOriginalColors } from '../../../experiments/model/status/colors'
-import {
-  DEFAULT_SECTION_NAMES,
-  CheckpointPlotsData,
-  PlotSize,
-  Section
-} from '../../../plots/webview/contract'
+import { CheckpointPlotsData, PlotSize } from '../../../plots/webview/contract'
 
 const colors = copyOriginalColors()
 
@@ -89,8 +84,7 @@ const data: CheckpointPlotsData = {
     'summary.json:val_loss',
     'summary.json:val_accuracy'
   ],
-  size: PlotSize.REGULAR,
-  sectionName: DEFAULT_SECTION_NAMES[Section.CHECKPOINT_PLOTS]
+  size: PlotSize.REGULAR
 }
 
 export const manyCheckpointPlots = (length: number) =>

extension/src/test/fixtures/plotsDiff/index.ts

@@ -5,11 +5,9 @@ import { PlotsOutput } from '../../../cli/reader'
 import {
   ComparisonPlots,
   ComparisonRevisionData,
-  DEFAULT_SECTION_NAMES,
   TemplatePlotSection,
   PlotSize,
   PlotsType,
-  Section,
   TemplatePlotGroup,
   TemplatePlotsData,
   TemplatePlots
@@ -531,14 +529,12 @@ export const getRevisions = () => {
 
 export const getMinimalWebviewMessage = () => ({
   plots: extendedSpecs(basicVega),
-  sectionName: DEFAULT_SECTION_NAMES[Section.TEMPLATE_PLOTS],
   size: PlotSize.REGULAR,
   revisions: getRevisions()
 })
 
 export const getTemplateWebviewMessage = (): TemplatePlotsData => ({
   plots: extendedSpecs({ ...basicVega, ...require('./vega').default }),
-  sectionName: DEFAULT_SECTION_NAMES[Section.TEMPLATE_PLOTS],
   size: PlotSize.REGULAR
 })
 
@@ -548,7 +544,6 @@ export const getManyTemplatePlotsWebviewMessage = (
   plots: extendedSpecs({
     ...multipleVega(length)
   }),
-  sectionName: DEFAULT_SECTION_NAMES[Section.TEMPLATE_PLOTS],
   size: PlotSize.REGULAR
 })
 
@@ -574,7 +569,6 @@ export const getComparisonWebviewMessage = (
 
   return {
     plots: plotAcc,
-    sectionName: DEFAULT_SECTION_NAMES[Section.COMPARISON_TABLE],
     size: PlotSize.REGULAR
   }
 }

extension/src/test/suite/plots/index.test.ts

@@ -384,37 +384,6 @@ suite('Plots Test Suite', () => {
       )
     }).timeout(WEBVIEW_TEST_TIMEOUT)
 
-    it('should handle a section renamed message from the webview', async () => {
-      const { plots, plotsModel } = await buildPlots(disposable)
-
-      const webview = await plots.showWebview()
-
-      const mockSendTelemetryEvent = stub(Telemetry, 'sendTelemetryEvent')
-      const mockMessageReceived = getMessageReceivedEmitter(webview)
-
-      const mockSetSectionName = stub(plotsModel, 'setSectionName').returns(
-        undefined
-      )
-      const mockName = 'some cool section name'
-
-      mockMessageReceived.fire({
-        payload: { name: mockName, section: Section.TEMPLATE_PLOTS },
-        type: MessageFromWebviewType.RENAME_SECTION
-      })
-
-      expect(mockSetSectionName).to.be.calledOnce
-      expect(mockSetSectionName).to.be.calledWithExactly(
-        Section.TEMPLATE_PLOTS,
-        mockName
-      )
-      expect(mockSendTelemetryEvent).to.be.calledOnce
-      expect(mockSendTelemetryEvent).to.be.calledWithExactly(
-        EventName.VIEWS_PLOTS_RENAME_SECTION,
-        { section: Section.TEMPLATE_PLOTS },
-        undefined
-      )
-    }).timeout(WEBVIEW_TEST_TIMEOUT)
-
     it('should handle a comparison revisions reordered message from the webview', async () => {
       const { plots, plotsModel, messageSpy } = await buildPlots(
         disposable,

extension/src/test/suite/plots/util.ts

@@ -95,7 +95,7 @@ export const getExpectedCheckpointPlotsData = (
   domain: string[],
   range: Color[]
 ) => {
-  const { plots, sectionName, selectedMetrics, size } = checkpointPlotsFixture
+  const { plots, selectedMetrics, size } = checkpointPlotsFixture
   return {
     checkpoint: {
       colors: {
@@ -106,7 +106,6 @@ export const getExpectedCheckpointPlotsData = (
         title: plot.title,
         values: plot.values.filter(values => domain.includes(values.group))
       })),
-      sectionName,
       selectedMetrics,
       size
     }