From 435d8d47650e68a448b677213132f0029fe39900 Mon Sep 17 00:00:00 2001 From: Riley Jones Date: Tue, 27 Dec 2022 20:02:51 +0000 Subject: [PATCH 1/6] add three new tooltip sort orders --- tensorboard/webapp/metrics/BUILD | 1 + tensorboard/webapp/metrics/internal_types.ts | 13 +------- .../webapp/metrics/store/metrics_reducers.ts | 18 +--------- .../scalar_card_component.ng.html | 5 +-- .../card_renderer/scalar_card_component.ts | 33 +++++++++++++++++-- .../right_pane/settings_view_component.ts | 3 ++ .../persistent_settings/_data_source/types.ts | 19 +++++++++-- .../line_chart_interactive_view.ng.html | 3 +- .../sub_view/line_chart_interactive_view.ts | 5 +++ 9 files changed, 63 insertions(+), 37 deletions(-) diff --git a/tensorboard/webapp/metrics/BUILD b/tensorboard/webapp/metrics/BUILD index 603556cce1..97e8170ed5 100644 --- a/tensorboard/webapp/metrics/BUILD +++ b/tensorboard/webapp/metrics/BUILD @@ -46,6 +46,7 @@ tf_ts_library( ], visibility = ["//tensorboard/webapp/metrics:__subpackages__"], deps = [ + "//tensorboard/webapp/persistent_settings/_data_source:types", "//tensorboard/webapp/widgets/card_fob:types", "//tensorboard/webapp/widgets/histogram:types", ], diff --git a/tensorboard/webapp/metrics/internal_types.ts b/tensorboard/webapp/metrics/internal_types.ts index 0df0f47e0c..78f4a30e4b 100644 --- a/tensorboard/webapp/metrics/internal_types.ts +++ b/tensorboard/webapp/metrics/internal_types.ts @@ -15,6 +15,7 @@ limitations under the License. import {TimeSelection} from '../widgets/card_fob/card_fob_types'; import {HistogramMode} from '../widgets/histogram/histogram_types'; +export {TooltipSort} from '../persistent_settings/_data_source/types'; export {HistogramMode, TimeSelection}; export enum PluginType { @@ -23,18 +24,6 @@ export enum PluginType { IMAGES = 'images', } -// When adding a new value to the enum, please implement the deserializer on -// data_source/metrics_data_source.ts. -// When editing a value of the enum, please write a backward compatible -// deserializer in data_source/metrics_data_source.ts. -export enum TooltipSort { - DEFAULT = 'default', - ALPHABETICAL = 'alphabetical', - ASCENDING = 'ascending', - DESCENDING = 'descending', - NEAREST = 'nearest', -} - export enum XAxisType { STEP, RELATIVE, diff --git a/tensorboard/webapp/metrics/store/metrics_reducers.ts b/tensorboard/webapp/metrics/store/metrics_reducers.ts index 760ef90215..3e3362065f 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers.ts @@ -42,7 +42,6 @@ import { CardUniqueInfo, SCALARS_SMOOTHING_MAX, SCALARS_SMOOTHING_MIN, - TooltipSort, URLDeserializedState, } from '../internal_types'; import {groupCardIdWithMetdata} from '../utils'; @@ -427,22 +426,7 @@ const reducer = createReducer( on(globalSettingsLoaded, (state, {partialSettings}) => { const metricsSettings: Partial = {}; if (partialSettings.tooltipSortString) { - switch (partialSettings.tooltipSortString) { - case TooltipSort.DEFAULT: - case TooltipSort.ALPHABETICAL: - metricsSettings.tooltipSort = TooltipSort.ALPHABETICAL; - break; - case TooltipSort.ASCENDING: - metricsSettings.tooltipSort = TooltipSort.ASCENDING; - break; - case TooltipSort.DESCENDING: - metricsSettings.tooltipSort = TooltipSort.DESCENDING; - break; - case TooltipSort.NEAREST: - metricsSettings.tooltipSort = TooltipSort.NEAREST; - break; - default: - } + metricsSettings.tooltipSort = partialSettings.tooltipSortString; } if (typeof partialSettings.timeSeriesCardMinWidth === 'number') { metricsSettings.cardMinWidth = partialSettings.timeSeriesCardMinWidth; diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html index 9a44004c0c..a4da6182d8 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html @@ -119,7 +119,8 @@ @@ -136,7 +137,7 @@ diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts index 97400854aa..6932386af6 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts @@ -163,7 +163,8 @@ export class ScalarCardComponent { getCursorAwareTooltipData( tooltipData: TooltipDatum[], - cursorLoc: {x: number; y: number} + cursorLocationInDataCoord: {x: number; y: number}, + cursorLocation: {x: number; y: number} ): ScalarTooltipDatum[] { const scalarTooltipData = tooltipData.map((datum) => { return { @@ -172,8 +173,20 @@ export class ScalarCardComponent { ...datum.metadata, closest: false, distSqToCursor: Math.hypot( - datum.point.x - cursorLoc.x, - datum.point.y - cursorLoc.y + datum.point.x - cursorLocationInDataCoord.x, + datum.point.y - cursorLocationInDataCoord.y + ), + distSqToCursorPixels: Math.hypot( + datum.point.x - cursorLocation.x, + datum.point.y - cursorLocation.y + ), + distSqToCursorX: Math.hypot( + datum.point.x - cursorLocationInDataCoord.x, + 0 + ), + distSqToCursorY: Math.hypot( + 0, + datum.point.y - cursorLocationInDataCoord.y ), }, }; @@ -201,6 +214,20 @@ export class ScalarCardComponent { return scalarTooltipData.sort((a, b) => { return a.metadata.distSqToCursor - b.metadata.distSqToCursor; }); + case TooltipSort.NEAREST_PIXEL: + return scalarTooltipData.sort((a, b) => { + return ( + a.metadata.distSqToCursorPixels - b.metadata.distSqToCursorPixels + ); + }); + case TooltipSort.NEAREST_X: + return scalarTooltipData.sort((a, b) => { + return a.metadata.distSqToCursorX - b.metadata.distSqToCursorX; + }); + case TooltipSort.NEAREST_Y: + return scalarTooltipData.sort((a, b) => { + return a.metadata.distSqToCursorY - b.metadata.distSqToCursorY; + }); case TooltipSort.DEFAULT: case TooltipSort.ALPHABETICAL: return scalarTooltipData.sort((a, b) => { diff --git a/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ts b/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ts index c3ca9cc252..fa5e00ee6a 100644 --- a/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ts +++ b/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ts @@ -87,6 +87,9 @@ export class SettingsViewComponent { {value: TooltipSort.ASCENDING, displayText: 'Ascending'}, {value: TooltipSort.DESCENDING, displayText: 'Descending'}, {value: TooltipSort.NEAREST, displayText: 'Nearest'}, + {value: TooltipSort.NEAREST_PIXEL, displayText: 'Nearest Pixel'}, + {value: TooltipSort.NEAREST_X, displayText: 'Nearest X'}, + {value: TooltipSort.NEAREST_Y, displayText: 'Nearest Y'}, ]; @Input() tooltipSort!: TooltipSort; @Output() tooltipSortChanged = new EventEmitter(); diff --git a/tensorboard/webapp/persistent_settings/_data_source/types.ts b/tensorboard/webapp/persistent_settings/_data_source/types.ts index 3df55ef954..0f2004713d 100644 --- a/tensorboard/webapp/persistent_settings/_data_source/types.ts +++ b/tensorboard/webapp/persistent_settings/_data_source/types.ts @@ -19,6 +19,21 @@ export enum ThemeValue { DARK = 'dark', } +// When adding a new value to the enum, please implement the deserializer on +// data_source/metrics_data_source.ts. +// When editing a value of the enum, please write a backward compatible +// deserializer in tensorboard/webapp/metrics/store/metrics_reducers.ts +export enum TooltipSort { + DEFAULT = 'default', + ALPHABETICAL = 'alphabetical', + ASCENDING = 'ascending', + DESCENDING = 'descending', + NEAREST = 'nearest', + NEAREST_PIXEL = 'nearest_pixel', + NEAREST_X = 'nearest_x', + NEAREST_Y = 'nearest_Y', +} + /** * Global settings that the backend remembers. `declare`d so properties do not * get mangled or mangled differently when a version compiler changes. @@ -28,7 +43,7 @@ export enum ThemeValue { */ export declare interface BackendSettings { scalarSmoothing?: number; - tooltipSort?: string; + tooltipSort?: TooltipSort; ignoreOutliers?: boolean; autoReload?: boolean; autoReloadPeriodInMs?: number; @@ -50,7 +65,7 @@ export declare interface BackendSettings { */ export interface PersistableSettings { scalarSmoothing?: number; - tooltipSortString?: string; + tooltipSortString?: TooltipSort; ignoreOutliers?: boolean; autoReload?: boolean; autoReloadPeriodInMs?: number; diff --git a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_interactive_view.ng.html b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_interactive_view.ng.html index c184103159..a75b8c8345 100644 --- a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_interactive_view.ng.html +++ b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_interactive_view.ng.html @@ -67,7 +67,8 @@ [ngTemplateOutlet]="tooltipTemplate ? tooltipTemplate : defaultTooltip" [ngTemplateOutletContext]="{ data: cursoredData, - cursorLocationInDataCoord: cursorLocationInDataCoord + cursorLocationInDataCoord: cursorLocationInDataCoord, + cursorLocation: cursorLocation }" > diff --git a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_interactive_view.ts b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_interactive_view.ts index 460c4c4035..87efbd5925 100644 --- a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_interactive_view.ts +++ b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_interactive_view.ts @@ -202,6 +202,7 @@ export class LineChartInteractiveViewComponent ]; cursorLocationInDataCoord: {x: number; y: number} | null = null; + cursorLocation: {x: number; y: number} | null = null; cursoredData: TooltipDatum[] = []; tooltipDisplayAttached: boolean = false; @@ -522,6 +523,10 @@ export class LineChartInteractiveViewComponent x: this.getDataX(event.offsetX), y: this.getDataY(event.offsetY), }; + this.cursorLocation = { + x: event.offsetX, + y: event.offsetY, + }; this.updateCursoredDataAndTooltipVisibility(); } From eb5123d092ac939bed73489dac2c81953cbd8ec6 Mon Sep 17 00:00:00 2001 From: Riley Jones Date: Tue, 3 Jan 2023 21:27:44 +0000 Subject: [PATCH 2/6] respond to code review comments --- .../webapp/metrics/store/metrics_reducers.ts | 6 ++- .../metrics/store/metrics_reducers_test.ts | 4 +- .../card_renderer/scalar_card_component.ts | 36 +++++--------- .../views/card_renderer/scalar_card_test.ts | 48 +++++++++++++++---- .../right_pane/settings_view_component.ts | 3 +- .../persistent_settings_data_source_test.ts | 11 +++-- .../persistent_settings/_data_source/types.ts | 1 - .../sub_view/line_chart_interactive_view.ts | 8 +++- 8 files changed, 73 insertions(+), 44 deletions(-) diff --git a/tensorboard/webapp/metrics/store/metrics_reducers.ts b/tensorboard/webapp/metrics/store/metrics_reducers.ts index 3e3362065f..b57c613a8e 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers.ts @@ -42,6 +42,7 @@ import { CardUniqueInfo, SCALARS_SMOOTHING_MAX, SCALARS_SMOOTHING_MIN, + TooltipSort, URLDeserializedState, } from '../internal_types'; import {groupCardIdWithMetdata} from '../utils'; @@ -425,7 +426,10 @@ const reducer = createReducer( }), on(globalSettingsLoaded, (state, {partialSettings}) => { const metricsSettings: Partial = {}; - if (partialSettings.tooltipSortString) { + if ( + partialSettings.tooltipSortString && + Object.values(TooltipSort).includes(partialSettings.tooltipSortString) + ) { metricsSettings.tooltipSort = partialSettings.tooltipSortString; } if (typeof partialSettings.timeSeriesCardMinWidth === 'number') { diff --git a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts index efbca27cf7..4ab57b96bb 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts @@ -2359,7 +2359,7 @@ describe('metrics reducers', () => { globalSettingsLoaded({ partialSettings: { ignoreOutliers: true, - tooltipSortString: 'descending', + tooltipSortString: 'descending' as TooltipSort, }, }) ); @@ -2387,7 +2387,7 @@ describe('metrics reducers', () => { beforeState, globalSettingsLoaded({ partialSettings: { - tooltipSortString: 'yo', + tooltipSortString: 'yo' as TooltipSort, }, }) ); diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts index 6932386af6..fb6a75e11f 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts @@ -58,7 +58,7 @@ import {TimeSelectionView} from './utils'; type ScalarTooltipDatum = TooltipDatum< ScalarCardSeriesMetadata & { - distSqToCursor: number; + distToCursor: number; closest: boolean; } >; @@ -172,22 +172,16 @@ export class ScalarCardComponent { metadata: { ...datum.metadata, closest: false, - distSqToCursor: Math.hypot( + distToCursor: Math.hypot( datum.point.x - cursorLocationInDataCoord.x, datum.point.y - cursorLocationInDataCoord.y ), - distSqToCursorPixels: Math.hypot( - datum.point.x - cursorLocation.x, - datum.point.y - cursorLocation.y - ), - distSqToCursorX: Math.hypot( - datum.point.x - cursorLocationInDataCoord.x, - 0 - ), - distSqToCursorY: Math.hypot( - 0, - datum.point.y - cursorLocationInDataCoord.y + distToCursorPixels: Math.hypot( + datum.pixelLocation.x - cursorLocation.x, + datum.pixelLocation.y - cursorLocation.y ), + distToCursorX: datum.point.x - cursorLocationInDataCoord.x, + distToCursorY: datum.point.y - cursorLocationInDataCoord.y, }, }; }); @@ -195,8 +189,8 @@ export class ScalarCardComponent { let minDist = Infinity; let minIndex = 0; for (let index = 0; index < scalarTooltipData.length; index++) { - if (minDist > scalarTooltipData[index].metadata.distSqToCursor) { - minDist = scalarTooltipData[index].metadata.distSqToCursor; + if (minDist > scalarTooltipData[index].metadata.distToCursor) { + minDist = scalarTooltipData[index].metadata.distToCursor; minIndex = index; } } @@ -212,21 +206,15 @@ export class ScalarCardComponent { return scalarTooltipData.sort((a, b) => b.point.y - a.point.y); case TooltipSort.NEAREST: return scalarTooltipData.sort((a, b) => { - return a.metadata.distSqToCursor - b.metadata.distSqToCursor; - }); - case TooltipSort.NEAREST_PIXEL: - return scalarTooltipData.sort((a, b) => { - return ( - a.metadata.distSqToCursorPixels - b.metadata.distSqToCursorPixels - ); + return a.metadata.distToCursorPixels - b.metadata.distToCursorPixels; }); case TooltipSort.NEAREST_X: return scalarTooltipData.sort((a, b) => { - return a.metadata.distSqToCursorX - b.metadata.distSqToCursorX; + return a.metadata.distToCursorX - b.metadata.distToCursorX; }); case TooltipSort.NEAREST_Y: return scalarTooltipData.sort((a, b) => { - return a.metadata.distSqToCursorY - b.metadata.distSqToCursorY; + return a.metadata.distToCursorY - b.metadata.distToCursorY; }); case TooltipSort.DEFAULT: case TooltipSort.ALPHABETICAL: diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts index 1d2d0612b1..3fe3ad0ba8 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts @@ -67,6 +67,7 @@ import { import { DataSeries, DataSeriesMetadataMap, + Point, RendererType, ScaleType, TooltipDatum, @@ -115,7 +116,8 @@ import {VisLinkedTimeSelectionWarningModule} from './vis_linked_time_selection_w [ngTemplateOutlet]="tooltipTemplate" [ngTemplateOutletContext]="{ data: tooltipDataForTesting, - cursorLocationInDataCoord: cursorLocForTesting + cursorLocationInDataCoord: cursorLocationInDataCoordForTesting, + cursorLocation: cursorLocationForTesting }" > (); - // This input does not exist on real line-chart and is devised to make tooltipTemplate + // These inputs do not exist on the real line-chart and is devised to make tooltipTemplate // testable without using the real implementation. @Input() tooltipDataForTesting: TooltipDatum[] = []; - @Input() cursorLocForTesting: {x: number; y: number} = {x: 0, y: 0}; + @Input() cursorLocationInDataCoordForTesting: {x: number; y: number} = { + x: 0, + y: 0, + }; + @Input() cursorLocationForTesting: {x: number; y: number} = {x: 0, y: 0}; private isViewBoxOverridden = new ReplaySubject(1); @@ -1142,7 +1148,8 @@ describe('scalar card', () => { describe('tooltip', () => { function buildTooltipDatum( metadata?: ScalarCardSeriesMetadata, - point: Partial = {} + point: Partial = {}, + pixelLocation: Point = {x: 0, y: 0} ): TooltipDatum { return { id: metadata?.id ?? 'a', @@ -1165,6 +1172,7 @@ describe('scalar card', () => { relativeTimeInMs: 0, ...point, }, + pixelLocation, }; } @@ -1180,11 +1188,19 @@ describe('scalar card', () => { function setCursorLocation( fixture: ComponentFixture, - cursorLocInDataCoord?: {x: number; y: number} + cursorLocInDataCoord?: {x: number; y: number}, + cursorLocationInPixels?: Point ) { const lineChart = fixture.debugElement.query(Selector.LINE_CHART); - lineChart.componentInstance.cursorLocForTesting = cursorLocInDataCoord; + if (cursorLocInDataCoord) { + lineChart.componentInstance.cursorLocationInDataCoordForTesting = + cursorLocInDataCoord; + } + if (cursorLocationInPixels) { + lineChart.componentInstance.cursorLocationForTesting = + cursorLocationInPixels; + } lineChart.componentInstance.changeDetectorRef.markForCheck(); } @@ -1616,6 +1632,10 @@ describe('scalar card', () => { y: 1000, value: 1000, wallTime: new Date('2020-01-01').getTime(), + }, + { + x: 0, + y: 100, } ), buildTooltipDatum( @@ -1634,6 +1654,10 @@ describe('scalar card', () => { y: -500, value: -500, wallTime: new Date('2020-12-31').getTime(), + }, + { + x: 50, + y: 0, } ), buildTooltipDatum( @@ -1652,10 +1676,14 @@ describe('scalar card', () => { y: 3, value: 3, wallTime: new Date('2021-01-01').getTime(), + }, + { + x: 1000, + y: 30, } ), ]); - setCursorLocation(fixture, {x: 500, y: -100}); + setCursorLocation(fixture, {x: 500, y: -100}, {x: 50, y: 0}); fixture.detectChanges(); assertTooltipRows(fixture, [ ['', 'Row 2', '-500', '1,000', anyString, anyString], @@ -1663,7 +1691,7 @@ describe('scalar card', () => { ['', 'Row 3', '3', '10,000', anyString, anyString], ]); - setCursorLocation(fixture, {x: 500, y: 600}); + setCursorLocation(fixture, {x: 500, y: 600}, {x: 50, y: 80}); fixture.detectChanges(); assertTooltipRows(fixture, [ ['', 'Row 1', '1000', '0', anyString, anyString], @@ -1671,7 +1699,7 @@ describe('scalar card', () => { ['', 'Row 3', '3', '10,000', anyString, anyString], ]); - setCursorLocation(fixture, {x: 10000, y: -100}); + setCursorLocation(fixture, {x: 10000, y: -100}, {x: 1000, y: 20}); fixture.detectChanges(); assertTooltipRows(fixture, [ ['', 'Row 3', '3', '10,000', anyString, anyString], @@ -1680,7 +1708,7 @@ describe('scalar card', () => { ]); // Right between row 1 and row 2. When tied, original order is used. - setCursorLocation(fixture, {x: 500, y: 250}); + setCursorLocation(fixture, {x: 500, y: 250}, {x: 25, y: 50}); fixture.detectChanges(); assertTooltipRows(fixture, [ ['', 'Row 1', '1000', '0', anyString, anyString], diff --git a/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ts b/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ts index fa5e00ee6a..38f0767e72 100644 --- a/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ts +++ b/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ts @@ -86,8 +86,7 @@ export class SettingsViewComponent { {value: TooltipSort.ALPHABETICAL, displayText: 'Alphabetical'}, {value: TooltipSort.ASCENDING, displayText: 'Ascending'}, {value: TooltipSort.DESCENDING, displayText: 'Descending'}, - {value: TooltipSort.NEAREST, displayText: 'Nearest'}, - {value: TooltipSort.NEAREST_PIXEL, displayText: 'Nearest Pixel'}, + {value: TooltipSort.NEAREST, displayText: 'Nearest Pixel'}, {value: TooltipSort.NEAREST_X, displayText: 'Nearest X'}, {value: TooltipSort.NEAREST_Y, displayText: 'Nearest Y'}, ]; diff --git a/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source_test.ts b/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source_test.ts index 5cda2e2c36..34ca9cd2a5 100644 --- a/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source_test.ts +++ b/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source_test.ts @@ -21,7 +21,12 @@ import { SettingsConverter, TEST_ONLY, } from './persistent_settings_data_source'; -import {BackendSettings, PersistableSettings, ThemeValue} from './types'; +import { + BackendSettings, + PersistableSettings, + ThemeValue, + TooltipSort, +} from './types'; describe('persistent_settings data_source test', () => { let getItemSpy: jasmine.Spy; @@ -289,7 +294,7 @@ describe('persistent_settings data_source test', () => { expect(actual).toEqual({ scalarSmoothing: 0.3, - tooltipSortString: 'ascending', + tooltipSortString: 'ascending' as TooltipSort, notificationLastReadTimeInMs: 3, }); }); @@ -311,7 +316,7 @@ describe('persistent_settings data_source test', () => { expect(actual).toEqual({ scalarSmoothing: 0.5, - tooltipSortString: 'default', + tooltipSortString: 'default' as TooltipSort, notificationLastReadTimeInMs: 100, }); }); diff --git a/tensorboard/webapp/persistent_settings/_data_source/types.ts b/tensorboard/webapp/persistent_settings/_data_source/types.ts index 0f2004713d..16160e6081 100644 --- a/tensorboard/webapp/persistent_settings/_data_source/types.ts +++ b/tensorboard/webapp/persistent_settings/_data_source/types.ts @@ -29,7 +29,6 @@ export enum TooltipSort { ASCENDING = 'ascending', DESCENDING = 'descending', NEAREST = 'nearest', - NEAREST_PIXEL = 'nearest_pixel', NEAREST_X = 'nearest_x', NEAREST_Y = 'nearest_Y', } diff --git a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_interactive_view.ts b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_interactive_view.ts index 87efbd5925..af4e1ad4aa 100644 --- a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_interactive_view.ts +++ b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_interactive_view.ts @@ -68,6 +68,7 @@ export interface TooltipDatum< metadata: Metadata; closestPointIndex: number; point: PointDatum; + pixelLocation: Point; } const SCROLL_ZOOM_SPEED_FACTOR = 0.01; @@ -555,10 +556,15 @@ export class LineChartInteractiveViewComponent }) .map(({seriesDatum, metadata}) => { const index = findClosestIndex(seriesDatum.points, cursorLoc.x); + const point = seriesDatum.points[index]; return { id: seriesDatum.id, closestPointIndex: index, - point: seriesDatum.points[index], + point, + pixelLocation: { + x: this.getDomX(point.x), + y: this.getDomY(point.y), + }, metadata, }; }) From edf212539ea5dde07d798224804f37fe08b481bf Mon Sep 17 00:00:00 2001 From: Riley Jones Date: Wed, 4 Jan 2023 18:05:15 +0000 Subject: [PATCH 3/6] remove nearest_x, rename tooltipSortSting --- tensorboard/webapp/metrics/metrics_module.ts | 2 +- tensorboard/webapp/metrics/store/metrics_reducers.ts | 6 +++--- tensorboard/webapp/metrics/store/metrics_reducers_test.ts | 4 ++-- .../views/card_renderer/scalar_card_component.ng.html | 3 ++- .../metrics/views/card_renderer/scalar_card_component.ts | 4 ---- .../metrics/views/right_pane/settings_view_component.ts | 1 - .../_data_source/persistent_settings_data_source.ts | 6 +++--- .../_data_source/persistent_settings_data_source_test.ts | 4 ++-- .../webapp/persistent_settings/_data_source/types.ts | 3 +-- 9 files changed, 14 insertions(+), 19 deletions(-) diff --git a/tensorboard/webapp/metrics/metrics_module.ts b/tensorboard/webapp/metrics/metrics_module.ts index bd80c8ac42..bac6530cd0 100644 --- a/tensorboard/webapp/metrics/metrics_module.ts +++ b/tensorboard/webapp/metrics/metrics_module.ts @@ -83,7 +83,7 @@ export function getMetricsIgnoreOutliersSettingFactory() { export function getMetricsTooltipSortSettingFactory() { return createSelector(getMetricsTooltipSort, (tooltipSort) => { - return {tooltipSortString: String(tooltipSort)}; + return {tooltipSort: String(tooltipSort)}; }); } diff --git a/tensorboard/webapp/metrics/store/metrics_reducers.ts b/tensorboard/webapp/metrics/store/metrics_reducers.ts index b57c613a8e..74bdcf5b32 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers.ts @@ -427,10 +427,10 @@ const reducer = createReducer( on(globalSettingsLoaded, (state, {partialSettings}) => { const metricsSettings: Partial = {}; if ( - partialSettings.tooltipSortString && - Object.values(TooltipSort).includes(partialSettings.tooltipSortString) + partialSettings.tooltipSort && + Object.values(TooltipSort).includes(partialSettings.tooltipSort) ) { - metricsSettings.tooltipSort = partialSettings.tooltipSortString; + metricsSettings.tooltipSort = partialSettings.tooltipSort; } if (typeof partialSettings.timeSeriesCardMinWidth === 'number') { metricsSettings.cardMinWidth = partialSettings.timeSeriesCardMinWidth; diff --git a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts index 4ab57b96bb..07d71a0dd9 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts @@ -2359,7 +2359,7 @@ describe('metrics reducers', () => { globalSettingsLoaded({ partialSettings: { ignoreOutliers: true, - tooltipSortString: 'descending' as TooltipSort, + tooltipSort: 'descending' as TooltipSort, }, }) ); @@ -2387,7 +2387,7 @@ describe('metrics reducers', () => { beforeState, globalSettingsLoaded({ partialSettings: { - tooltipSortString: 'yo' as TooltipSort, + tooltipSort: 'yo' as TooltipSort, }, }) ); diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html index a4da6182d8..8b39534761 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html @@ -137,7 +137,8 @@ diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts index fb6a75e11f..6dc82db0ce 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts @@ -208,10 +208,6 @@ export class ScalarCardComponent { return scalarTooltipData.sort((a, b) => { return a.metadata.distToCursorPixels - b.metadata.distToCursorPixels; }); - case TooltipSort.NEAREST_X: - return scalarTooltipData.sort((a, b) => { - return a.metadata.distToCursorX - b.metadata.distToCursorX; - }); case TooltipSort.NEAREST_Y: return scalarTooltipData.sort((a, b) => { return a.metadata.distToCursorY - b.metadata.distToCursorY; diff --git a/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ts b/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ts index 38f0767e72..a212f05a75 100644 --- a/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ts +++ b/tensorboard/webapp/metrics/views/right_pane/settings_view_component.ts @@ -87,7 +87,6 @@ export class SettingsViewComponent { {value: TooltipSort.ASCENDING, displayText: 'Ascending'}, {value: TooltipSort.DESCENDING, displayText: 'Descending'}, {value: TooltipSort.NEAREST, displayText: 'Nearest Pixel'}, - {value: TooltipSort.NEAREST_X, displayText: 'Nearest X'}, {value: TooltipSort.NEAREST_Y, displayText: 'Nearest Y'}, ]; @Input() tooltipSort!: TooltipSort; diff --git a/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source.ts b/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source.ts index 0ee706328f..99417fb3d9 100644 --- a/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source.ts +++ b/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source.ts @@ -51,10 +51,10 @@ export class OSSSettingsConverter extends SettingsConverter< if (settings.scalarSmoothing !== undefined) { serializableSettings.scalarSmoothing = settings.scalarSmoothing; } - if (settings.tooltipSortString !== undefined) { + if (settings.tooltipSort !== undefined) { // TooltipSort is a string enum and has string values; no need to // serialize it differently to account for their unintended changes. - serializableSettings.tooltipSort = settings.tooltipSortString; + serializableSettings.tooltipSort = settings.tooltipSort; } if (settings.autoReload !== undefined) { serializableSettings.autoReload = settings.autoReload; @@ -117,7 +117,7 @@ export class OSSSettingsConverter extends SettingsConverter< backendSettings.hasOwnProperty('tooltipSort') && typeof backendSettings.tooltipSort === 'string' ) { - settings.tooltipSortString = backendSettings.tooltipSort; + settings.tooltipSort = backendSettings.tooltipSort; } if ( diff --git a/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source_test.ts b/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source_test.ts index 34ca9cd2a5..874b00125c 100644 --- a/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source_test.ts +++ b/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source_test.ts @@ -294,7 +294,7 @@ describe('persistent_settings data_source test', () => { expect(actual).toEqual({ scalarSmoothing: 0.3, - tooltipSortString: 'ascending' as TooltipSort, + tooltipSort: 'ascending' as TooltipSort, notificationLastReadTimeInMs: 3, }); }); @@ -316,7 +316,7 @@ describe('persistent_settings data_source test', () => { expect(actual).toEqual({ scalarSmoothing: 0.5, - tooltipSortString: 'default' as TooltipSort, + tooltipSort: 'default' as TooltipSort, notificationLastReadTimeInMs: 100, }); }); diff --git a/tensorboard/webapp/persistent_settings/_data_source/types.ts b/tensorboard/webapp/persistent_settings/_data_source/types.ts index 16160e6081..af04bcfa93 100644 --- a/tensorboard/webapp/persistent_settings/_data_source/types.ts +++ b/tensorboard/webapp/persistent_settings/_data_source/types.ts @@ -29,7 +29,6 @@ export enum TooltipSort { ASCENDING = 'ascending', DESCENDING = 'descending', NEAREST = 'nearest', - NEAREST_X = 'nearest_x', NEAREST_Y = 'nearest_Y', } @@ -64,7 +63,7 @@ export declare interface BackendSettings { */ export interface PersistableSettings { scalarSmoothing?: number; - tooltipSortString?: TooltipSort; + tooltipSort?: TooltipSort; ignoreOutliers?: boolean; autoReload?: boolean; autoReloadPeriodInMs?: number; From 7140374f3d0b15ae0b8b164c095a6c54dabded3d Mon Sep 17 00:00:00 2001 From: Riley Jones Date: Wed, 4 Jan 2023 18:42:53 +0000 Subject: [PATCH 4/6] update closest logic --- .../metrics/views/card_renderer/scalar_card_component.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts index 6dc82db0ce..fabed7d842 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts @@ -58,7 +58,6 @@ import {TimeSelectionView} from './utils'; type ScalarTooltipDatum = TooltipDatum< ScalarCardSeriesMetadata & { - distToCursor: number; closest: boolean; } >; @@ -172,10 +171,6 @@ export class ScalarCardComponent { metadata: { ...datum.metadata, closest: false, - distToCursor: Math.hypot( - datum.point.x - cursorLocationInDataCoord.x, - datum.point.y - cursorLocationInDataCoord.y - ), distToCursorPixels: Math.hypot( datum.pixelLocation.x - cursorLocation.x, datum.pixelLocation.y - cursorLocation.y @@ -189,8 +184,8 @@ export class ScalarCardComponent { let minDist = Infinity; let minIndex = 0; for (let index = 0; index < scalarTooltipData.length; index++) { - if (minDist > scalarTooltipData[index].metadata.distToCursor) { - minDist = scalarTooltipData[index].metadata.distToCursor; + if (minDist > scalarTooltipData[index].metadata.distToCursorPixels) { + minDist = scalarTooltipData[index].metadata.distToCursorPixels; minIndex = index; } } From e3ce6e70234c35f1dea67d6d47d764a886894ae7 Mon Sep 17 00:00:00 2001 From: Riley Jones Date: Fri, 6 Jan 2023 22:24:43 +0000 Subject: [PATCH 5/6] resolve circular dependency issue --- WORKSPACE | 4 ++-- tensorboard/webapp/metrics/BUILD | 2 -- tensorboard/webapp/metrics/actions/BUILD | 2 +- tensorboard/webapp/metrics/actions/index.ts | 2 +- tensorboard/webapp/metrics/internal_types.ts | 1 - tensorboard/webapp/metrics/store/BUILD | 6 +++--- .../webapp/metrics/store/metrics_reducers.ts | 2 +- .../webapp/metrics/store/metrics_reducers_test.ts | 14 +++++++------- .../webapp/metrics/store/metrics_selectors.ts | 2 +- .../metrics/store/metrics_selectors_test.ts | 2 +- tensorboard/webapp/metrics/store/metrics_types.ts | 2 +- tensorboard/webapp/metrics/types.ts | 13 +++++++++++++ .../card_renderer/scalar_card_component.ng.html | 2 +- .../webapp/persistent_settings/_data_source/BUILD | 5 +++++ .../persistent_settings_data_source_test.ts | 8 ++------ .../persistent_settings/_data_source/types.ts | 15 ++------------- 16 files changed, 41 insertions(+), 41 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 16312d0997..e28981714d 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -24,11 +24,11 @@ http_archive( load("@bazel_skylib//lib:versions.bzl", "versions") versions.check( + # TODO(https://github.com/tensorflow/tensorboard/issues/6115): Support building TensorBoard with Bazel version >= 6.0.0 + maximum_bazel_version = "5.4.0", # Keep this version in sync with: # * The BAZEL environment variable defined in .github/workflows/ci.yml, which is used for CI and nightly builds. minimum_bazel_version = "4.2.2", - # TODO(https://github.com/tensorflow/tensorboard/issues/6115): Support building TensorBoard with Bazel version >= 6.0.0 - maximum_bazel_version = "5.4.0", ) http_archive( diff --git a/tensorboard/webapp/metrics/BUILD b/tensorboard/webapp/metrics/BUILD index 97e8170ed5..ddf5208910 100644 --- a/tensorboard/webapp/metrics/BUILD +++ b/tensorboard/webapp/metrics/BUILD @@ -35,7 +35,6 @@ tf_ts_library( ], deps = [ ":internal_types", - "//tensorboard/webapp/metrics/store:types", ], ) @@ -46,7 +45,6 @@ tf_ts_library( ], visibility = ["//tensorboard/webapp/metrics:__subpackages__"], deps = [ - "//tensorboard/webapp/persistent_settings/_data_source:types", "//tensorboard/webapp/widgets/card_fob:types", "//tensorboard/webapp/widgets/histogram:types", ], diff --git a/tensorboard/webapp/metrics/actions/BUILD b/tensorboard/webapp/metrics/actions/BUILD index eb45146603..873f281de0 100644 --- a/tensorboard/webapp/metrics/actions/BUILD +++ b/tensorboard/webapp/metrics/actions/BUILD @@ -10,7 +10,7 @@ tf_ts_library( "index.ts", ], deps = [ - "//tensorboard/webapp/metrics:internal_types", + "//tensorboard/webapp/metrics:types", "//tensorboard/webapp/metrics/data_source", "//tensorboard/webapp/metrics/views/card_renderer:scalar_card_types", "//tensorboard/webapp/util:dom", diff --git a/tensorboard/webapp/metrics/actions/index.ts b/tensorboard/webapp/metrics/actions/index.ts index 594ba6fdd2..8edd99aa9a 100644 --- a/tensorboard/webapp/metrics/actions/index.ts +++ b/tensorboard/webapp/metrics/actions/index.ts @@ -29,7 +29,7 @@ import { PluginType, TooltipSort, XAxisType, -} from '../internal_types'; +} from '../types'; import { ColumnHeader, SortingInfo, diff --git a/tensorboard/webapp/metrics/internal_types.ts b/tensorboard/webapp/metrics/internal_types.ts index 78f4a30e4b..ccec830d21 100644 --- a/tensorboard/webapp/metrics/internal_types.ts +++ b/tensorboard/webapp/metrics/internal_types.ts @@ -15,7 +15,6 @@ limitations under the License. import {TimeSelection} from '../widgets/card_fob/card_fob_types'; import {HistogramMode} from '../widgets/histogram/histogram_types'; -export {TooltipSort} from '../persistent_settings/_data_source/types'; export {HistogramMode, TimeSelection}; export enum PluginType { diff --git a/tensorboard/webapp/metrics/store/BUILD b/tensorboard/webapp/metrics/store/BUILD index d627a40e1f..957a12812c 100644 --- a/tensorboard/webapp/metrics/store/BUILD +++ b/tensorboard/webapp/metrics/store/BUILD @@ -18,7 +18,7 @@ tf_ts_library( "//tensorboard/webapp/app_routing:types", "//tensorboard/webapp/app_routing/actions", "//tensorboard/webapp/core/actions", - "//tensorboard/webapp/metrics:internal_types", + "//tensorboard/webapp/metrics:types", "//tensorboard/webapp/metrics:utils", "//tensorboard/webapp/metrics/actions", "//tensorboard/webapp/metrics/data_source", @@ -68,7 +68,7 @@ tf_ts_library( ], deps = [ "//tensorboard/webapp/app_routing:namespaced_state_reducer_helper", - "//tensorboard/webapp/metrics:internal_types", + "//tensorboard/webapp/metrics:types", "//tensorboard/webapp/metrics/data_source", "//tensorboard/webapp/metrics/views/card_renderer:scalar_card_types", "//tensorboard/webapp/types", @@ -92,8 +92,8 @@ tf_ts_library( "//tensorboard/webapp/app_routing:types", "//tensorboard/webapp/app_routing/actions", "//tensorboard/webapp/core/actions", - "//tensorboard/webapp/metrics:internal_types", "//tensorboard/webapp/metrics:test_lib", + "//tensorboard/webapp/metrics:types", "//tensorboard/webapp/metrics/actions", "//tensorboard/webapp/metrics/data_source", "//tensorboard/webapp/persistent_settings", diff --git a/tensorboard/webapp/metrics/store/metrics_reducers.ts b/tensorboard/webapp/metrics/store/metrics_reducers.ts index 74bdcf5b32..092747afa6 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers.ts @@ -44,7 +44,7 @@ import { SCALARS_SMOOTHING_MIN, TooltipSort, URLDeserializedState, -} from '../internal_types'; +} from '../types'; import {groupCardIdWithMetdata} from '../utils'; import {ColumnHeaderType} from '../views/card_renderer/scalar_card_types'; import { diff --git a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts index 07d71a0dd9..60f2940cb1 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts @@ -26,13 +26,6 @@ import { ScalarStepDatum, TagMetadata as DataSourceTagMetadata, } from '../data_source'; -import { - CardId, - CardMetadata, - HistogramMode, - TooltipSort, - XAxisType, -} from '../internal_types'; import { buildDataSourceTagMetadata, buildMetricsSettingsState, @@ -46,6 +39,13 @@ import { createScalarStepData, createTimeSeriesData, } from '../testing'; +import { + CardId, + CardMetadata, + HistogramMode, + TooltipSort, + XAxisType, +} from '../types'; import {reducers} from './metrics_reducers'; import {getCardId, getPinnedCardId} from './metrics_store_internal_utils'; import { diff --git a/tensorboard/webapp/metrics/store/metrics_selectors.ts b/tensorboard/webapp/metrics/store/metrics_selectors.ts index 4378309d46..3cd02061ac 100644 --- a/tensorboard/webapp/metrics/store/metrics_selectors.ts +++ b/tensorboard/webapp/metrics/store/metrics_selectors.ts @@ -29,7 +29,7 @@ import { TimeSelection, TooltipSort, XAxisType, -} from '../internal_types'; +} from '../types'; import {ColumnHeader} from '../views/card_renderer/scalar_card_types'; import * as storeUtils from './metrics_store_internal_utils'; import { diff --git a/tensorboard/webapp/metrics/store/metrics_selectors_test.ts b/tensorboard/webapp/metrics/store/metrics_selectors_test.ts index 6abdc96765..e0eae51bb7 100644 --- a/tensorboard/webapp/metrics/store/metrics_selectors_test.ts +++ b/tensorboard/webapp/metrics/store/metrics_selectors_test.ts @@ -15,7 +15,6 @@ limitations under the License. import {DataLoadState} from '../../types/data'; import {nextElementId} from '../../util/dom'; import {PluginType} from '../data_source'; -import {HistogramMode, TooltipSort, XAxisType} from '../internal_types'; import { appStateFromMetricsState, buildMetricsSettingsState, @@ -26,6 +25,7 @@ import { createScalarStepData, createTimeSeriesData, } from '../testing'; +import {HistogramMode, TooltipSort, XAxisType} from '../types'; import * as selectors from './metrics_selectors'; describe('metrics selectors', () => { diff --git a/tensorboard/webapp/metrics/store/metrics_types.ts b/tensorboard/webapp/metrics/store/metrics_types.ts index 90cefbf70b..88f73c1bb1 100644 --- a/tensorboard/webapp/metrics/store/metrics_types.ts +++ b/tensorboard/webapp/metrics/store/metrics_types.ts @@ -35,7 +35,7 @@ import { TimeSelection, TooltipSort, XAxisType, -} from '../internal_types'; +} from '../types'; import {ColumnHeader} from '../views/card_renderer/scalar_card_types'; export const METRICS_FEATURE_KEY = 'metrics'; diff --git a/tensorboard/webapp/metrics/types.ts b/tensorboard/webapp/metrics/types.ts index bc2af3751a..350694ef8d 100644 --- a/tensorboard/webapp/metrics/types.ts +++ b/tensorboard/webapp/metrics/types.ts @@ -13,3 +13,16 @@ See the License for the specific language governing permissions and limitations under the License. ==============================================================================*/ export * from './internal_types'; + +// When adding a new value to the enum, please implement the deserializer on +// data_source/metrics_data_source.ts. +// When editing a value of the enum, please write a backward compatible +// deserializer in tensorboard/webapp/metrics/store/metrics_reducers.ts +export enum TooltipSort { + DEFAULT = 'default', + ALPHABETICAL = 'alphabetical', + ASCENDING = 'ascending', + DESCENDING = 'descending', + NEAREST = 'nearest', + NEAREST_Y = 'nearest_Y', +} diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html index 8b39534761..744a043300 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html @@ -137,7 +137,7 @@ { let getItemSpy: jasmine.Spy; diff --git a/tensorboard/webapp/persistent_settings/_data_source/types.ts b/tensorboard/webapp/persistent_settings/_data_source/types.ts index af04bcfa93..2a9d1a4aaa 100644 --- a/tensorboard/webapp/persistent_settings/_data_source/types.ts +++ b/tensorboard/webapp/persistent_settings/_data_source/types.ts @@ -13,25 +13,14 @@ See the License for the specific language governing permissions and limitations under the License. ==============================================================================*/ +import {TooltipSort} from '../../metrics/types'; + export enum ThemeValue { BROWSER_DEFAULT = 'browser_default', LIGHT = 'light', DARK = 'dark', } -// When adding a new value to the enum, please implement the deserializer on -// data_source/metrics_data_source.ts. -// When editing a value of the enum, please write a backward compatible -// deserializer in tensorboard/webapp/metrics/store/metrics_reducers.ts -export enum TooltipSort { - DEFAULT = 'default', - ALPHABETICAL = 'alphabetical', - ASCENDING = 'ascending', - DESCENDING = 'descending', - NEAREST = 'nearest', - NEAREST_Y = 'nearest_Y', -} - /** * Global settings that the backend remembers. `declare`d so properties do not * get mangled or mangled differently when a version compiler changes. From 6d270282fea1060c3525ceeca7b8959f2c60a8e8 Mon Sep 17 00:00:00 2001 From: Riley Jones Date: Mon, 9 Jan 2023 22:19:00 +0000 Subject: [PATCH 6/6] rename variables, remove workspace file --- WORKSPACE | 4 ++-- .../metrics/store/metrics_reducers_test.ts | 2 +- .../scalar_card_component.ng.html | 11 +++++---- .../card_renderer/scalar_card_component.ts | 12 +++++----- .../views/card_renderer/scalar_card_test.ts | 24 +++++++++---------- .../persistent_settings_data_source_test.ts | 2 +- .../line_chart_interactive_view.ng.html | 10 ++++---- .../sub_view/line_chart_interactive_view.ts | 14 +++++------ 8 files changed, 39 insertions(+), 40 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index e28981714d..16312d0997 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -24,11 +24,11 @@ http_archive( load("@bazel_skylib//lib:versions.bzl", "versions") versions.check( - # TODO(https://github.com/tensorflow/tensorboard/issues/6115): Support building TensorBoard with Bazel version >= 6.0.0 - maximum_bazel_version = "5.4.0", # Keep this version in sync with: # * The BAZEL environment variable defined in .github/workflows/ci.yml, which is used for CI and nightly builds. minimum_bazel_version = "4.2.2", + # TODO(https://github.com/tensorflow/tensorboard/issues/6115): Support building TensorBoard with Bazel version >= 6.0.0 + maximum_bazel_version = "5.4.0", ) http_archive( diff --git a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts index 60f2940cb1..f3990a2285 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts @@ -2359,7 +2359,7 @@ describe('metrics reducers', () => { globalSettingsLoaded({ partialSettings: { ignoreOutliers: true, - tooltipSort: 'descending' as TooltipSort, + tooltipSort: TooltipSort.DESCENDING, }, }) ); diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html index 744a043300..814e03a266 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html @@ -155,14 +155,15 @@ >{{ datum.metadata.displayName }} - + - - + + diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts index fabed7d842..420c208bfd 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts @@ -172,11 +172,11 @@ export class ScalarCardComponent { ...datum.metadata, closest: false, distToCursorPixels: Math.hypot( - datum.pixelLocation.x - cursorLocation.x, - datum.pixelLocation.y - cursorLocation.y + datum.domPoint.x - cursorLocation.x, + datum.domPoint.y - cursorLocation.y ), - distToCursorX: datum.point.x - cursorLocationInDataCoord.x, - distToCursorY: datum.point.y - cursorLocationInDataCoord.y, + distToCursorX: datum.dataPoint.x - cursorLocationInDataCoord.x, + distToCursorY: datum.dataPoint.y - cursorLocationInDataCoord.y, }, }; }); @@ -196,9 +196,9 @@ export class ScalarCardComponent { switch (this.tooltipSort) { case TooltipSort.ASCENDING: - return scalarTooltipData.sort((a, b) => a.point.y - b.point.y); + return scalarTooltipData.sort((a, b) => a.dataPoint.y - b.dataPoint.y); case TooltipSort.DESCENDING: - return scalarTooltipData.sort((a, b) => b.point.y - a.point.y); + return scalarTooltipData.sort((a, b) => b.dataPoint.y - a.dataPoint.y); case TooltipSort.NEAREST: return scalarTooltipData.sort((a, b) => { return a.metadata.distToCursorPixels - b.metadata.distToCursorPixels; diff --git a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts index 3fe3ad0ba8..db4c71e386 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts @@ -1148,8 +1148,8 @@ describe('scalar card', () => { describe('tooltip', () => { function buildTooltipDatum( metadata?: ScalarCardSeriesMetadata, - point: Partial = {}, - pixelLocation: Point = {x: 0, y: 0} + dataPoint: Partial = {}, + domPoint: Point = {x: 0, y: 0} ): TooltipDatum { return { id: metadata?.id ?? 'a', @@ -1163,16 +1163,16 @@ describe('scalar card', () => { ...metadata, }, closestPointIndex: 0, - point: { + dataPoint: { x: 0, y: 0, value: 0, step: 0, wallTime: 0, relativeTimeInMs: 0, - ...point, + ...dataPoint, }, - pixelLocation, + domPoint, }; } @@ -1188,18 +1188,16 @@ describe('scalar card', () => { function setCursorLocation( fixture: ComponentFixture, - cursorLocInDataCoord?: {x: number; y: number}, - cursorLocationInPixels?: Point + dataPoint?: {x: number; y: number}, + domPoint?: Point ) { const lineChart = fixture.debugElement.query(Selector.LINE_CHART); - if (cursorLocInDataCoord) { - lineChart.componentInstance.cursorLocationInDataCoordForTesting = - cursorLocInDataCoord; + if (dataPoint) { + lineChart.componentInstance.dataPointForTesting = dataPoint; } - if (cursorLocationInPixels) { - lineChart.componentInstance.cursorLocationForTesting = - cursorLocationInPixels; + if (domPoint) { + lineChart.componentInstance.cursorLocationForTesting = domPoint; } lineChart.componentInstance.changeDetectorRef.markForCheck(); } diff --git a/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source_test.ts b/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source_test.ts index 4a5753cd8a..caf77bcef3 100644 --- a/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source_test.ts +++ b/tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source_test.ts @@ -312,7 +312,7 @@ describe('persistent_settings data_source test', () => { expect(actual).toEqual({ scalarSmoothing: 0.5, - tooltipSort: 'default' as TooltipSort, + tooltipSort: TooltipSort.DEFAULT, notificationLastReadTimeInMs: 100, }); }); diff --git a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_interactive_view.ng.html b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_interactive_view.ng.html index a75b8c8345..bc451ed4f8 100644 --- a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_interactive_view.ng.html +++ b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_interactive_view.ng.html @@ -26,9 +26,9 @@ *ngFor="let datum of cursoredData; trackBy: trackBySeriesName" > @@ -93,8 +93,8 @@ - - + + diff --git a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_interactive_view.ts b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_interactive_view.ts index af4e1ad4aa..5d3eebaa2e 100644 --- a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_interactive_view.ts +++ b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_interactive_view.ts @@ -67,8 +67,8 @@ export interface TooltipDatum< id: string; metadata: Metadata; closestPointIndex: number; - point: PointDatum; - pixelLocation: Point; + dataPoint: PointDatum; + domPoint: Point; } const SCROLL_ZOOM_SPEED_FACTOR = 0.01; @@ -556,14 +556,14 @@ export class LineChartInteractiveViewComponent }) .map(({seriesDatum, metadata}) => { const index = findClosestIndex(seriesDatum.points, cursorLoc.x); - const point = seriesDatum.points[index]; + const dataPoint = seriesDatum.points[index]; return { id: seriesDatum.id, closestPointIndex: index, - point, - pixelLocation: { - x: this.getDomX(point.x), - y: this.getDomY(point.y), + dataPoint, + domPoint: { + x: this.getDomX(dataPoint.x), + y: this.getDomY(dataPoint.y), }, metadata, };
- {{ valueFormatter.formatShort(datum.point.y) }} + {{ valueFormatter.formatShort(datum.dataPoint.y) }} {{ valueFormatter.formatShort(datum.point.value) }}{{ valueFormatter.formatShort(datum.dataPoint.value) }} {{ stepFormatter.formatShort(datum.point.step) }}{{ datum.point.wallTime | date: 'short' }}{{ stepFormatter.formatShort(datum.dataPoint.step) }}{{ datum.dataPoint.wallTime | date: 'short' }} - {{ relativeXFormatter.formatReadable(datum.point.relativeTimeInMs) + {{ + relativeXFormatter.formatReadable(datum.dataPoint.relativeTimeInMs) }}
{{ datum.metadata.displayName }}{{ datum.point.y }}{{ datum.point.x }}{{ datum.dataPoint.y }}{{ datum.dataPoint.x }}