Apply table filters to custom plot data (#4657)

Author: julieg18

extension/src/experiments/index.ts

@@ -382,6 +382,10 @@ export class Experiments extends BaseRepository<TableData> {
     return this.experiments.getWorkspaceCommitsAndExperiments()
   }
 
+  public getUnfilteredCommitsAndExperiments() {
+    return this.experiments.getUnfilteredCommitsAndExperiments()
+  }
+
   public async selectExperimentsToPlot() {
     const experiments = this.experiments.getWorkspaceCommitsAndExperiments()
 

extension/src/experiments/model/index.ts

@@ -1,6 +1,6 @@
 import { Memento } from 'vscode'
 import { SortDefinition, sortExperiments } from './sortBy'
-import { FilterDefinition, getFilterId } from './filterBy'
+import { FilterDefinition, filterExperiment, getFilterId } from './filterBy'
 import { collectFiltered, collectUnfiltered } from './filterBy/collect'
 import {
   collectAddRemoveCommitsDetails,
@@ -365,6 +365,14 @@ export class ExperimentsModel extends ModelWithPersistence {
     return [...this.getWorkspaceAndCommits(), ...this.getExperiments()]
   }
 
+  public getUnfilteredCommitsAndExperiments() {
+    const filters = this.getFilters()
+    return this.getWorkspaceCommitsAndExperiments().filter(
+      exp =>
+        exp.id !== EXPERIMENT_WORKSPACE_ID && !!filterExperiment(filters, exp)
+    )
+  }
+
   public getErrors() {
     const errors = new Set<string>()
     for (const { error, label } of this.getCombinedList()) {

extension/src/plots/model/index.ts

@@ -27,7 +27,6 @@ import {
   DEFAULT_SECTION_NB_ITEMS_PER_ROW_OR_WIDTH,
   PlotsSection,
   SectionCollapsed,
-  CustomPlotData,
   CustomPlotsData,
   DEFAULT_HEIGHT,
   DEFAULT_NB_ITEMS_PER_ROW,
@@ -154,44 +153,40 @@ export class PlotsModel extends ModelWithPersistence {
   }
 
   public getCustomPlots(): CustomPlotsData | undefined {
-    const experiments = this.experiments
-      .getWorkspaceCommitsAndExperiments()
-      .filter(({ id }) => id !== EXPERIMENT_WORKSPACE_ID)
-
-    if (experiments.length === 0) {
-      return
-    }
-
+    const experiments = this.experiments.getUnfilteredCommitsAndExperiments()
+    const hasUnfilteredExperiments = experiments.length > 0
+    const plotsOrderValues = this.getCustomPlotsOrder()
+    const enablePlotCreation = checkForCustomPlotOptions(
+      this.experiments.getColumnTerminalNodes(),
+      plotsOrderValues
+    )
     const height = this.getHeight(PlotsSection.CUSTOM_PLOTS)
     const nbItemsPerRow = this.getNbItemsPerRowOrWidth(
       PlotsSection.CUSTOM_PLOTS
     )
+    const hasAddedPlots = plotsOrderValues.length > 0
+
     const colorScale = getColorScale(
       this.getSelectedRevisionDetails().filter(
         ({ id }) => id !== EXPERIMENT_WORKSPACE_ID
       )
     )
-    const plotsOrderValues = this.getCustomPlotsOrder()
-    const plots: CustomPlotData[] = collectCustomPlots({
-      colorScale,
-      experiments,
-      height,
-      nbItemsPerRow,
-      plotsOrderValues
-    })
-
-    if (plots.length === 0 && plotsOrderValues.length > 0) {
-      return
-    }
 
     return {
-      enablePlotCreation: checkForCustomPlotOptions(
-        this.experiments.getColumnTerminalNodes(),
-        plotsOrderValues
-      ),
+      enablePlotCreation,
+      hasAddedPlots,
+      hasUnfilteredExperiments,
       height,
       nbItemsPerRow,
-      plots
+      plots: hasUnfilteredExperiments
+        ? collectCustomPlots({
+            colorScale,
+            experiments,
+            height,
+            nbItemsPerRow,
+            plotsOrderValues
+          })
+        : []
     }
   }
 

extension/src/plots/webview/contract.ts

@@ -102,6 +102,8 @@ export type CustomPlotsData = {
   nbItemsPerRow: number
   enablePlotCreation: boolean
   height: PlotHeight
+  hasUnfilteredExperiments: boolean
+  hasAddedPlots: boolean
 }
 
 export enum PlotsType {

extension/src/test/fixtures/expShow/base/customPlots.ts

@@ -313,7 +313,9 @@ const data: CustomPlotsData = {
     }
   ],
   nbItemsPerRow: DEFAULT_NB_ITEMS_PER_ROW,
-  height: DEFAULT_PLOT_HEIGHT
+  height: DEFAULT_PLOT_HEIGHT,
+  hasAddedPlots: true,
+  hasUnfilteredExperiments: true
 }
 
 export default data

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

@@ -44,9 +44,16 @@ import { isErrorItem } from '../../../tree'
 import { RegisteredCommands } from '../../../commands/external'
 import { REVISIONS } from '../../fixtures/plotsDiff'
 import * as FileSystem from '../../../fileSystem'
-import { ExpShowOutput, experimentHasError } from '../../../cli/dvc/contract'
+import {
+  EXPERIMENT_WORKSPACE_ID,
+  ExpShowOutput,
+  experimentHasError
+} from '../../../cli/dvc/contract'
+import { Experiment } from '../../../experiments/webview/contract'
 import { COMMITS_SEPARATOR } from '../../../cli/git/constants'
 import { BaseWebview } from '../../../webview'
+import * as PlotsCollectUtils from '../../../plots/model/collect'
+import { Operator } from '../../../experiments/model/filterBy'
 
 suite('Plots Test Suite', () => {
   const disposable = Disposable.fn()
@@ -172,6 +179,84 @@ suite('Plots Test Suite', () => {
       )
     })
 
+    describe('Custom Plots Creation', () => {
+      it('should only use unfiltered experiments and commits in custom plots', async () => {
+        const { plots, plotsModel, experimentsModel } = await buildPlots({
+          disposer: disposable,
+          plotsDiff: plotsDiffFixture
+        })
+
+        const plotsCustomPlotsSpy = spy(PlotsCollectUtils, 'collectCustomPlots')
+
+        await plots.isReady()
+
+        stub(experimentsModel, 'getFilters')
+          .onFirstCall()
+          .returns([
+            {
+              operator: Operator.EQUAL,
+              path: 'params:params.yaml:epochs',
+              value: 2
+            }
+          ])
+
+        plotsModel.getCustomPlots()
+
+        const allExperiments: Experiment[] =
+          experimentsModel.getWorkspaceCommitsAndExperiments()
+
+        const { experiments } = plotsCustomPlotsSpy.firstCall.args[0]
+
+        expect(experiments).to.deep.equal(
+          allExperiments.filter(
+            ({ id, params }) =>
+              id !== EXPERIMENT_WORKSPACE_ID &&
+              params?.['params.yaml']?.epochs === 2
+          )
+        )
+      })
+
+      it('should handle all experiments/commits being filtered', async () => {
+        const { plots, plotsModel, experimentsModel } = await buildPlots({
+          disposer: disposable,
+          plotsDiff: plotsDiffFixture
+        })
+
+        await plots.isReady()
+
+        stub(experimentsModel, 'getUnfilteredCommitsAndExperiments')
+          .onFirstCall()
+          .returns([])
+
+        const customPlots = plotsModel.getCustomPlots()
+
+        expect(customPlots).to.deep.equal({
+          ...customPlotsFixture,
+          hasUnfilteredExperiments: false,
+          plots: []
+        })
+      })
+
+      it('should handle no plots being added yet', async () => {
+        const { plots, plotsModel } = await buildPlots({
+          disposer: disposable,
+          plotsDiff: plotsDiffFixture
+        })
+
+        await plots.isReady()
+
+        stub(plotsModel, 'getCustomPlotsOrder').onFirstCall().returns([])
+
+        const customPlots = plotsModel.getCustomPlots()
+
+        expect(customPlots).to.deep.equal({
+          ...customPlotsFixture,
+          hasAddedPlots: false,
+          plots: []
+        })
+      })
+    })
+
     it('should handle a section resized message from the webview', async () => {
       const { mockMessageReceived, plotsModel } = await buildPlotsWebview({
         disposer: disposable

webview/src/plots/components/App.test.tsx

@@ -533,6 +533,7 @@ describe('App', () => {
     renderAppWithOptionalData({
       custom: {
         ...customPlotsFixture,
+        hasAddedPlots: false,
         plots: []
       },
       template: templatePlotsFixture
@@ -544,6 +545,22 @@ describe('App', () => {
     expect(screen.getByText('No Plots Added')).toBeInTheDocument()
   })
 
+  it('should render custom with "No Data to Plot" message when there are added plots but no unfiltered experiments', () => {
+    renderAppWithOptionalData({
+      custom: {
+        ...customPlotsFixture,
+        hasUnfilteredExperiments: false,
+        plots: []
+      },
+      template: templatePlotsFixture
+    })
+
+    expect(screen.queryByText('Loading Plots...')).not.toBeInTheDocument()
+    expect(screen.queryByText('No Plots to Display')).not.toBeInTheDocument()
+    expect(screen.getByText('Custom')).toBeInTheDocument()
+    expect(screen.getByText('No Data to Plot')).toBeInTheDocument()
+  })
+
   it('should render the comparison table when given a message with comparison plots data', () => {
     const expectedSectionName = 'Images'
 

webview/src/plots/components/customPlots/CustomPlots.tsx

@@ -22,9 +22,14 @@ interface CustomPlotsProps {
 
 export const CustomPlots: React.FC<CustomPlotsProps> = ({ plotsIds }) => {
   const [order, setOrder] = useState(plotsIds)
-  const { nbItemsPerRow, hasData, hasItems, disabledDragPlotIds } = useSelector(
-    (state: PlotsState) => state.custom
-  )
+  const {
+    nbItemsPerRow,
+    hasData,
+    hasItems,
+    hasAddedPlots,
+    disabledDragPlotIds,
+    hasUnfilteredExperiments
+  } = useSelector((state: PlotsState) => state.custom)
   const [onSection, setOnSection] = useState(false)
   const draggedRef = useSelector(
     (state: PlotsState) => state.dragAndDrop.draggedRef
@@ -50,10 +55,14 @@ export const CustomPlots: React.FC<CustomPlotsProps> = ({ plotsIds }) => {
     return <EmptyState isFullScreen={false}>No Plots to Display</EmptyState>
   }
 
-  if (order.length === 0) {
+  if (!hasAddedPlots) {
     return <EmptyState isFullScreen={false}>No Plots Added</EmptyState>
   }
 
+  if (!hasUnfilteredExperiments) {
+    return <EmptyState isFullScreen={false}>No Data to Plot</EmptyState>
+  }
+
   const items = order.map(plot => (
     <div key={plot} id={plot}>
       <CustomPlot id={plot} />

webview/src/plots/components/customPlots/CustomPlotsWrapper.tsx

@@ -8,8 +8,14 @@ import { PlotsState } from '../../store'
 import { addCustomPlot, removeCustomPlots } from '../../util/messages'
 
 export const CustomPlotsWrapper: React.FC = () => {
-  const { plotsIds, nbItemsPerRow, isCollapsed, height, enablePlotCreation } =
-    useSelector((state: PlotsState) => state.custom)
+  const {
+    plotsIds,
+    nbItemsPerRow,
+    isCollapsed,
+    height,
+    enablePlotCreation,
+    hasAddedPlots
+  } = useSelector((state: PlotsState) => state.custom)
   const [selectedPlots, setSelectedPlots] = useState<string[]>([])
   useEffect(() => {
     setSelectedPlots(plotsIds)
@@ -26,7 +32,9 @@ export const CustomPlotsWrapper: React.FC = () => {
       addPlotsButton={
         enablePlotCreation ? { onClick: addCustomPlot } : undefined
       }
-      removePlotsButton={hasItems ? { onClick: removeCustomPlots } : undefined}
+      removePlotsButton={
+        hasAddedPlots ? { onClick: removeCustomPlots } : undefined
+      }
       changeSize={changeSize}
       hasItems={hasItems}
       height={height}

webview/src/plots/components/customPlots/customPlotsSlice.ts

@@ -21,8 +21,10 @@ export interface CustomPlotsState extends Omit<CustomPlotsData, 'plots'> {
 export const customPlotsInitialState: CustomPlotsState = {
   disabledDragPlotIds: [],
   enablePlotCreation: true,
+  hasAddedPlots: false,
   hasData: false,
   hasItems: false,
+  hasUnfilteredExperiments: false,
   height: DEFAULT_HEIGHT[PlotsSection.CUSTOM_PLOTS],
   isCollapsed: DEFAULT_SECTION_COLLAPSED[PlotsSection.CUSTOM_PLOTS],
   nbItemsPerRow: