Skip to content

Commit 4164ca1

Browse files
authored
Continue moving to use name in SortingInfo (#6400)
## Motivation for features / changes For the HParam work that is being done we are now using the name field as the unique identifier for columns. Therefore the SortingInfo interface needs to use name and not header. However, making the hard swap to using name would break sync so it is happening in stages. The name switch to using names happened in #6362. However, we had to make name optional for sync reasons. Googlers see cl/534097086 to see where name was added. Now this PR makes header optional. I will then make an internal change to remove it. Then I can finally submit a final PR which removes the property entirely.
1 parent 94251bb commit 4164ca1

File tree

4 files changed

+3
-14
lines changed

4 files changed

+3
-14
lines changed

tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,6 @@ export class ScalarCardComponent<Downloader> {
123123
@ViewChild(LineChartComponent)
124124
lineChart?: LineChartComponent;
125125
sortingInfo: SortingInfo = {
126-
header: ColumnHeaderType.RUN, //This is no longer used but the type needs it or it will break sync. TODO(jameshollyer): remove this once internal code all uses name.
127126
name: 'run',
128127
order: SortingOrder.ASCENDING,
129128
};

tensorboard/webapp/widgets/data_table/data_table_component.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,14 +118,12 @@ export class DataTableComponent implements OnDestroy {
118118
this.sortingInfo.order === SortingOrder.ASCENDING
119119
) {
120120
this.sortDataBy.emit({
121-
header: ColumnHeaderType.RUN, //This is no longer used but the sortingInfo interface needs it or it will break sync. TODO(jameshollyer): remove this once internal code all uses name.
122121
name,
123122
order: SortingOrder.DESCENDING,
124123
});
125124
return;
126125
}
127126
this.sortDataBy.emit({
128-
header: ColumnHeaderType.RUN, //This is no longer used but the sortingInfo interface needs it or it will break sync. TODO(jameshollyer): remove this once internal code all uses name.
129127
name,
130128
order: SortingOrder.ASCENDING,
131129
});

tensorboard/webapp/widgets/data_table/data_table_test.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ describe('data table', () => {
7575
fixture.componentInstance.headers = input.headers || [];
7676
fixture.componentInstance.data = input.data || [];
7777
fixture.componentInstance.sortingInfo = input.sortingInfo || {
78-
header: ColumnHeaderType.RUN,
7978
name: 'run',
8079
order: SortingOrder.ASCENDING,
8180
};
@@ -416,7 +415,6 @@ describe('data table', () => {
416415

417416
headerElements[3].triggerEventHandler('click', {});
418417
expect(sortDataBySpy).toHaveBeenCalledOnceWith({
419-
header: jasmine.any(String), // This attribute is no longer used but temporarily left here to no break internal syncs
420418
name: 'step',
421419
order: SortingOrder.ASCENDING,
422420
});
@@ -451,7 +449,6 @@ describe('data table', () => {
451449
},
452450
],
453451
sortingInfo: {
454-
header: ColumnHeaderType.STEP,
455452
name: 'step',
456453
order: SortingOrder.ASCENDING,
457454
},
@@ -463,7 +460,6 @@ describe('data table', () => {
463460

464461
headerElements[3].triggerEventHandler('click', {});
465462
expect(sortDataBySpy).toHaveBeenCalledOnceWith({
466-
header: jasmine.any(String), // This attribute is no longer used but temporarily left here to no break internal syncs
467463
name: 'step',
468464
order: SortingOrder.DESCENDING,
469465
});
@@ -492,7 +488,6 @@ describe('data table', () => {
492488
},
493489
],
494490
sortingInfo: {
495-
header: ColumnHeaderType.VALUE,
496491
name: 'value',
497492
order: SortingOrder.ASCENDING,
498493
},
@@ -557,7 +552,6 @@ describe('data table', () => {
557552
},
558553
],
559554
sortingInfo: {
560-
header: ColumnHeaderType.STEP,
561555
name: 'step',
562556
order: SortingOrder.DESCENDING,
563557
},
@@ -622,7 +616,6 @@ describe('data table', () => {
622616
},
623617
],
624618
sortingInfo: {
625-
header: ColumnHeaderType.STEP,
626619
name: 'step',
627620
order: SortingOrder.DESCENDING,
628621
},
@@ -692,7 +685,6 @@ describe('data table', () => {
692685
},
693686
],
694687
sortingInfo: {
695-
header: ColumnHeaderType.STEP,
696688
name: 'step',
697689
order: SortingOrder.DESCENDING,
698690
},

tensorboard/webapp/widgets/data_table/types.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,9 @@ export interface SortingInfo {
5656
// Currently in the process of moving from header to name.
5757
// Header is no longer used but is required as to not break sync
5858
// TODO(jameshollyer): Remove header once all internal code is switched
59-
// to using name and make name required.
60-
header: ColumnHeaderType;
61-
name?: string;
59+
// to using name.
60+
header?: ColumnHeaderType;
61+
name: string;
6262
order: SortingOrder;
6363
}
6464

0 commit comments

Comments
 (0)