Skip to content

Commit

Permalink
Model Details: add editable properties section, set up API patches fo…
Browse files Browse the repository at this point in the history 
…r description, labels and properties

Signed-off-by: Mike Turley <[email protected]>
  • Loading branch information
@mturley
mturley committed May 3, 2024
1 parent 521f401 commit d04fcab
Show file tree
Hide file tree
Showing 12 changed files with 883 additions and 134 deletions.
44 changes: 22 additions & 22 deletions frontend/src/__mocks__/mockRegisteredModelsList.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable camelcase */
import { RegisteredModelList } from '~/concepts/modelRegistry/types';
import { ModelRegistryMetadataType, RegisteredModelList } from '~/concepts/modelRegistry/types';
import { mockRegisteredModel } from './mockRegisteredModel';

export const mockRegisteredModelList = ({
Expand All @@ -14,27 +14,27 @@ export const mockRegisteredModelList = ({
'A machine learning model trained to detect fraudulent transactions in financial data',
customProperties: {
Financial: {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: 'non-empty',
},
'Financial data': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Fraud detection': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Test label': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Machine learning': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Next data to be overflow': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
},
Expand All @@ -43,23 +43,23 @@ export const mockRegisteredModelList = ({
name: 'Credit Scoring',
customProperties: {
'Credit Score Predictor': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Creditworthiness scoring system': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Default Risk Analyzer': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Portfolio Management': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Risk Assessment': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
},
Expand All @@ -70,44 +70,44 @@ export const mockRegisteredModelList = ({
'A machine learning model trained to detect fraudulent transactions in financial data',
customProperties: {
'Testing label': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
Financial: {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: 'non-empty',
},
'Financial data': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Fraud detection': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Long label data to be truncated abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc':
{
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Machine learning': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Next data to be overflow': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Label x': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Label y': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Label z': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
},
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/DashboardDescriptionListGroup.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ const DashboardDescriptionListGroup: React.FC<DashboardDescriptionListGroupProps
</ActionListItem>
<ActionListItem>
<Button
data-testid={`discard-edit-button-${title} `}
data-testid={`discard-edit-button-${title}`}
aria-label={`Discard edits to ${title} `}
variant="plain"
onClick={onDiscardEditsClick}
Expand Down
33 changes: 26 additions & 7 deletions frontend/src/components/EditableLabelsDescriptionListGroup.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,16 @@ type EditableTextDescriptionListGroupProps = Partial<
Pick<DashboardDescriptionListGroupProps, 'title' | 'contentWhenEmpty'>
> & {
labels: string[];
saveEditedLabels: (labels: string[]) => Promise<void>;
saveEditedLabels: (labels: string[]) => Promise<unknown>;
allExistingKeys?: string[];
};

const EditableLabelsDescriptionListGroup: React.FC<EditableTextDescriptionListGroupProps> = ({
title = 'Labels',
contentWhenEmpty = 'No labels',
labels,
saveEditedLabels,
allExistingKeys = labels,
}) => {
const [isEditing, setIsEditing] = React.useState(false);
const [unsavedLabels, setUnsavedLabels] = React.useState(labels);
Expand All @@ -54,10 +56,22 @@ const EditableLabelsDescriptionListGroup: React.FC<EditableTextDescriptionListGr
setUnsavedLabels([...unsavedLabels, text]);
};

// Don't allow a label that matches a non-label property key or another label (as they stand before saving)
// Note that this means if you remove a label and add it back before saving, that is valid
const reservedKeys = [
...allExistingKeys.filter((key) => !labels.includes(key)),
...unsavedLabels,
];

const [isAddLabelModalOpen, setIsAddLabelModalOpen] = React.useState(false);
const [addLabelInputValue, setAddLabelInputValue] = React.useState('');
const addLabelInputRef = React.useRef<HTMLInputElement>(null);
const addLabelInputTooLong = addLabelInputValue.length > 63;
let addLabelValidationError: string | null = null;
if (reservedKeys.includes(addLabelInputValue)) {
addLabelValidationError = 'Label must not match an existing label or property key';
} else if (addLabelInputValue.length > 63) {
addLabelValidationError = "Label text can't exceed 63 characters";
}

const toggleAddLabelModal = () => {
setAddLabelInputValue('');
Expand All @@ -68,7 +82,8 @@ const EditableLabelsDescriptionListGroup: React.FC<EditableTextDescriptionListGr
addLabelInputRef.current.focus();
}
}, [isAddLabelModalOpen]);
const addLabelModalSubmitDisabled = !addLabelInputValue || addLabelInputTooLong;

const addLabelModalSubmitDisabled = !addLabelInputValue || !!addLabelValidationError;
const submitAddLabelModal = (event?: React.FormEvent) => {
event?.preventDefault();
if (!addLabelModalSubmitDisabled) {
Expand Down Expand Up @@ -107,7 +122,11 @@ const EditableLabelsDescriptionListGroup: React.FC<EditableTextDescriptionListGr
editableProps={{ 'aria-label': `Editable label with text ${label}` }}
onClose={() => removeUnsavedLabel(label)}
closeBtnProps={{ isDisabled: isSavingEdits }}
onEditComplete={(_event, newText) => editUnsavedLabel(newText, index)}
onEditComplete={(_event, newText) => {
if (!reservedKeys.includes(newText) && newText.length <= 63) {
editUnsavedLabel(newText, index);
}
}}
>
{label}
</Label>
Expand Down Expand Up @@ -172,13 +191,13 @@ const EditableLabelsDescriptionListGroup: React.FC<EditableTextDescriptionListGr
}
ref={addLabelInputRef}
isRequired
validated={addLabelInputTooLong ? 'error' : 'default'}
validated={addLabelValidationError ? 'error' : 'default'}
/>
{addLabelInputTooLong && (
{addLabelValidationError && (
<FormHelperText>
<HelperText>
<HelperTextItem icon={<ExclamationCircleIcon />} variant="error">
Label text can&apos;t exceed 63 characters
{addLabelValidationError}
</HelperTextItem>
</HelperText>
</FormHelperText>
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/components/EditableTextDescriptionListGroup.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ type EditableTextDescriptionListGroupProps = Pick<
'title' | 'contentWhenEmpty'
> & {
value: string;
saveEditedValue: (value: string) => Promise<void>;
saveEditedValue: (value: string) => Promise<unknown>;
};

const EditableTextDescriptionListGroup: React.FC<EditableTextDescriptionListGroupProps> = ({
Expand Down Expand Up @@ -37,6 +37,8 @@ const EditableTextDescriptionListGroup: React.FC<EditableTextDescriptionListGrou
value={unsavedValue}
onChange={(_event, v) => setUnsavedValue(v)}
isDisabled={isSavingEdits}
rows={24}
resizeOrientation="vertical"
/>
}
onEditClick={() => {
Expand Down
53 changes: 52 additions & 1 deletion frontend/src/concepts/modelRegistry/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,65 @@ export enum ModelArtifactState {
REFERENCE = 'REFERENCE',
}

export enum ModelRegistryMetadataType {
INT = 'MetadataIntValue',
DOUBLE = 'MetadataDoubleValue',
STRING = 'MetadataStringValue',
STRUCT = 'MetadataStructValue',
PROTO = 'MetadataProtoValue',
BOOL = 'MetadataBoolValue',
}

export type ModelRegistryCustomPropertyInt = {
metadataType: ModelRegistryMetadataType.INT;
int_value: string; // int64-formatted string
};

export type ModelRegistryCustomPropertyDouble = {
metadataType: ModelRegistryMetadataType.DOUBLE;
double_value: number;
};

export type ModelRegistryCustomPropertyString = {
metadataType: ModelRegistryMetadataType.STRING;
string_value: string;
};

export type ModelRegistryCustomPropertyStruct = {
metadataType: ModelRegistryMetadataType.STRUCT;
struct_value: string; // Base64 encoded bytes for struct value
};

export type ModelRegistryCustomPropertyProto = {
metadataType: ModelRegistryMetadataType.PROTO;
type: string; // url describing proto value
proto_value: string; // Base64 encoded bytes for proto value
};

export type ModelRegistryCustomPropertyBool = {
metadataType: ModelRegistryMetadataType.BOOL;
bool_value: boolean;
};

export type ModelRegistryCustomProperty =
| ModelRegistryCustomPropertyInt
| ModelRegistryCustomPropertyDouble
| ModelRegistryCustomPropertyString
| ModelRegistryCustomPropertyStruct
| ModelRegistryCustomPropertyProto
| ModelRegistryCustomPropertyBool;

export type ModelRegistryCustomProperties = Record<string, ModelRegistryCustomProperty>;
export type ModelRegistryStringCustomProperties = Record<string, ModelRegistryCustomPropertyString>;

export type ModelRegistryBase = {
id: string;
name: string;
externalID?: string;
description?: string;
createTimeSinceEpoch?: string;
lastUpdateTimeSinceEpoch: string;
customProperties: Record<string, Record<string, string>>;
customProperties: ModelRegistryCustomProperties;
};

export type ModelArtifact = ModelRegistryBase & {
Expand Down
Loading

0 comments on commit d04fcab

Please sign in to comment.