Skip to content

Commit

Permalink
Removes LitComponentLayout.
Browse files Browse the repository at this point in the history
* Removes Python struct and updates derived types in layout.py
* Removes TypeScript struct and canonicalization function in types.ts
* Removes calls to canonicalization function in state_service.ts
* Updates UTs as appropriate

PiperOrigin-RevId: 549619461
  • Loading branch information
RyanMullins authored and LIT team committed Jul 20, 2023
1 parent 4a5c0cb commit 5f6a46a
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 124 deletions.
19 changes: 1 addition & 18 deletions lit_nlp/api/layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,22 +92,6 @@ class LayoutSettings(dtypes.DataTuple):
centerPage: bool = False


@attr.s(auto_attribs=True)
class LitComponentLayout(dtypes.DataTuple):
"""Frontend UI layout (legacy); should match client/lib/types.ts."""
# Keys are names of tabs; one must be called "Main".
# Values are names of LitModule HTML elements,
# e.g. data-table-module for the DataTableModule class.
components: Dict[str, LitModuleList]
layoutSettings: LayoutSettings = attr.ib(factory=LayoutSettings)
description: Optional[str] = None

def to_json(self) -> JsonDict:
"""Override serialization to properly convert nested objects."""
# Not invertible, but these only go from server -> frontend anyway.
return attr.asdict(self, recurse=True)


@attr.s(auto_attribs=True)
class LitCanonicalLayout(dtypes.DataTuple):
"""Frontend UI layout; should match client/lib/types.ts."""
Expand All @@ -124,8 +108,7 @@ def to_json(self) -> JsonDict:
return attr.asdict(self, recurse=True)


LitComponentLayouts = Mapping[str, Union[LitComponentLayout,
LitCanonicalLayout]]
LitComponentLayouts = Mapping[str, LitCanonicalLayout]

# pylint: enable=invalid-name
# LINT.ThenChange(../client/lib/types.ts)
Expand Down
53 changes: 2 additions & 51 deletions lit_nlp/client/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,10 +340,10 @@ export function defaultValueByField(key: string, spec: Spec) {
}

/**
* Dictionary of lit layouts. See LitComponentLayout
* Dictionary of lit layouts. See LitCanonicalLayout
*/
export declare interface LitComponentLayouts {
[key: string]: LitComponentLayout|LitCanonicalLayout;
[key: string]: LitCanonicalLayout;
}

// LINT.IfChange
Expand Down Expand Up @@ -380,23 +380,6 @@ export declare interface LitTabGroupLayout {
[tabName: string]: LitComponentSpecifier[];
}

/**
* A layout is defined by a set of modules arranged into tabs and groups.
*
* This is a legacy layout format, where components contains several keys
* including 'Main', and values are lists of module classes.
* - The 'Main' group will appear in the upper half of the UI
* - The remaining groups will appear in the lower half of the UI,
* with a tab bar to select the active group.
*
* See layout.ts for examples.
*/
export declare interface LitComponentLayout {
components: LitTabGroupLayout;
layoutSettings?: LayoutSettings;
description?: string;
}

/**
* UI layout in canonical form.
*
Expand All @@ -422,38 +405,6 @@ export declare interface LayoutSettings {
centerPage?: boolean;
}

/**
* Convert a layout to canonical form.
* TODO(b/265218467): deprecate this once we convert all client and demo layouts.
* TODO(b/265218467): move this to Python.
*/
export function canonicalizeLayout(layout: LitComponentLayout|
LitCanonicalLayout): LitCanonicalLayout {
if (!layout.hasOwnProperty('components')) {
return layout as LitCanonicalLayout;
}
// Legacy layout to convert.
layout = layout as LitComponentLayout;

const canonicalLayout: LitCanonicalLayout = {
upper: {},
lower: {},
layoutSettings: layout.layoutSettings ?? {},
description: layout.description ?? '',
};

// Handle upper layout.
canonicalLayout.upper['Main'] = layout.components['Main'];

// Handle lower layout.
for (const tabName of Object.keys(layout.components)) {
if (tabName === 'Main') continue;
canonicalLayout.lower[tabName] = layout.components[tabName];
}

return canonicalLayout;
}

// LINT.ThenChange(../../api/layout.py)

/** Display name for the "no dataset" dataset in settings. */
Expand Down
51 changes: 1 addition & 50 deletions lit_nlp/client/lib/types_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,7 @@

import 'jasmine';

// These are needed to preserve the import, as they are referenced indirectly.
// through the names of elements they define.
// tslint:disable-next-line:no-unused-variable
import {ClassificationModule} from '../modules/classification_module';
// tslint:disable-next-line:no-unused-variable
import {DatapointEditorModule} from '../modules/datapoint_editor_module';

import {canonicalizeLayout, formatForDisplay, LitCanonicalLayout, LitComponentLayout} from './types';
import {formatForDisplay} from './types';

describe('formatForDisplay test', () => {

Expand All @@ -47,45 +40,3 @@ describe('formatForDisplay test', () => {
});
});
});

const MOCK_LAYOUT: LitComponentLayout = {
components: {
'Main': [
'datapoint-editor-module',
],
'internals': [
// Duplicated per model and in compareDatapoints mode.
'classification-module',
],
},
layoutSettings: {hideToolbar: true, mainHeight: 90, centerPage: true},
description: 'Mock layout for testing.'
};

const CANONICAL_LAYOUT: LitCanonicalLayout = {
upper: {
'Main': [
'datapoint-editor-module',
],
},
lower: {
'internals': [
// Duplicated per model and in compareDatapoints mode.
'classification-module',
],
},
layoutSettings: {hideToolbar: true, mainHeight: 90, centerPage: true},
description: 'Mock layout for testing.'
};

describe('canonicalizeLayout test', () => {
type LitLayout = LitComponentLayout|LitCanonicalLayout;
const tests: Array<[string, LitLayout, LitCanonicalLayout]> = [
['converts a legacy layout', MOCK_LAYOUT, CANONICAL_LAYOUT],
['leaves canonical layouts alone', CANONICAL_LAYOUT, CANONICAL_LAYOUT],
];

tests.forEach(([name, layout, expected]) => {
it(name, () => {expect(canonicalizeLayout(layout)).toEqual(expected);});
});
});
8 changes: 3 additions & 5 deletions lit_nlp/client/services/state_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import {action, computed, observable, toJS} from 'mobx';

import {FieldMatcher, ImageBytes} from '../lib/lit_types';
import {canonicalizeLayout, IndexedInput, LitCanonicalLayout, LitComponentLayouts, LitMetadata, ModelInfo, ModelInfoMap, ModelSpec, Spec} from '../lib/types';
import {IndexedInput, LitCanonicalLayout, LitComponentLayouts, LitMetadata, ModelInfo, ModelInfoMap, ModelSpec, Spec} from '../lib/types';
import {getTypes, findSpecKeys} from '../lib/utils';

import {ApiService} from './api_service';
Expand Down Expand Up @@ -61,7 +61,7 @@ export class AppState extends LitService implements StateObservedByUrlService {
@observable currentModels: string[] = [];
@observable compareExamplesEnabled = false;
@observable layoutName!: string;
@observable layouts: {[name: string]: LitCanonicalLayout} = {};
@observable layouts: LitComponentLayouts = {};
private readonly newDatapointsCallbacks: NewDatapointsFn[] = [];

@computed
Expand Down Expand Up @@ -293,9 +293,7 @@ export class AppState extends LitService implements StateObservedByUrlService {

//=================================== Initialization logic
addLayouts(layouts: LitComponentLayouts) {
for (const name of Object.keys(layouts)) {
this.layouts[name] = canonicalizeLayout(layouts[name]);
}
Object.assign(this.layouts, layouts);
}

@action
Expand Down

0 comments on commit 5f6a46a

Please sign in to comment.