-
Notifications
You must be signed in to change notification settings - Fork 179
feat(ui): register and display model_type from catalog and model registry #2545
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
a0d6444
d707a9a
9dfa838
15e0f3f
a13e1ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,25 +20,30 @@ import { | |
| DashboardDescriptionListGroup, | ||
| EditableLabelsDescriptionListGroup, | ||
| } from 'mod-arch-shared'; | ||
| import { RegisteredModel } from '~/app/types'; | ||
| import { ModelRegistryCustomProperties, RegisteredModel } from '~/app/types'; | ||
| import ModelTimestamp from '~/app/pages/modelRegistry/screens/components/ModelTimestamp'; | ||
| import ModelPropertiesExpandableSection from '~/app/pages/modelRegistry/screens/components/ModelPropertiesExpandableSection'; | ||
| import { ModelRegistryContext } from '~/app/context/ModelRegistryContext'; | ||
| import { getLabels, mergeUpdatedLabels } from '~/app/pages/modelRegistry/screens/utils'; | ||
| import { EMPTY_CUSTOM_PROPERTY_VALUE } from '~/concepts/modelCatalog/const'; | ||
| import { formatModelTypeDisplay } from '~/app/pages/modelCatalog/utils/modelCatalogUtils'; | ||
| import { getModelTypeRawStringFromCustomProperties } from '~/app/pages/modelRegistry/screens/RegisterModel/registerModelTypeUtils'; | ||
|
|
||
| type ModelDetailsCardProps = { | ||
| registeredModel: RegisteredModel; | ||
| refresh: () => void; | ||
| isArchiveModel?: boolean; | ||
| isExpandable?: boolean; | ||
| /** If `model_type` is absent on the registered model, read from these (e.g. version custom properties). */ | ||
| modelTypeFallbackCustomProperties?: ModelRegistryCustomProperties; | ||
| }; | ||
|
|
||
| const ModelDetailsCard: React.FC<ModelDetailsCardProps> = ({ | ||
| registeredModel: rm, | ||
| refresh, | ||
| isArchiveModel, | ||
| isExpandable, | ||
| modelTypeFallbackCustomProperties, | ||
| }) => { | ||
| const { apiState } = React.useContext(ModelRegistryContext); | ||
| const [isExpanded, setIsExpanded] = React.useState(false); | ||
|
|
@@ -112,6 +117,10 @@ const ModelDetailsCard: React.FC<ModelDetailsCardProps> = ({ | |
| /> | ||
| ); | ||
|
|
||
| const modelTypeRaw = | ||
| getModelTypeRawStringFromCustomProperties(rm.customProperties) ?? | ||
| getModelTypeRawStringFromCustomProperties(modelTypeFallbackCustomProperties); | ||
|
|
||
| const infoSection = ( | ||
| <> | ||
| <DashboardDescriptionListGroup | ||
|
|
@@ -146,6 +155,11 @@ const ModelDetailsCard: React.FC<ModelDetailsCardProps> = ({ | |
| > | ||
| <ModelTimestamp timeSinceEpoch={rm.createTimeSinceEpoch} /> | ||
| </DashboardDescriptionListGroup> | ||
| <DashboardDescriptionListGroup title="Model type"> | ||
| <Content component="p" data-testid="registered-model-model-type"> | ||
| {formatModelTypeDisplay(modelTypeRaw)} | ||
| </Content> | ||
|
Comment on lines
155
to
+161
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| </DashboardDescriptionListGroup> | ||
| </> | ||
| ); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| import { FormGroup } from '@patternfly/react-core'; | ||
| import React from 'react'; | ||
| import { SimpleSelect } from 'mod-arch-shared'; | ||
| import { SimpleSelectOption } from 'mod-arch-shared/dist/components/SimpleSelect'; | ||
| import { ModelRegistryCustomProperties } from '~/app/types'; | ||
| import { ModelType } from '~/concepts/modelCatalog/const'; | ||
| import { formatModelTypeDisplay } from '~/app/pages/modelCatalog/utils/modelCatalogUtils'; | ||
| import FormFieldset from '~/app/pages/modelRegistry/screens/components/FormFieldset'; | ||
| import { | ||
| buildCustomPropertiesWithModelType, | ||
| getModelTypeStoredValueFromCustomProperties, | ||
| } from './registerModelTypeUtils'; | ||
|
|
||
| const MODEL_TYPE_PLACEHOLDER_KEY = '__model-type-unset__'; | ||
|
|
||
| const MODEL_TYPE_SELECT_OPTIONS: SimpleSelectOption[] = [ | ||
| { | ||
| key: MODEL_TYPE_PLACEHOLDER_KEY, | ||
| label: 'Select model type', | ||
| isPlaceholder: true, | ||
| }, | ||
| { | ||
| key: ModelType.GENERATIVE, | ||
| label: formatModelTypeDisplay(ModelType.GENERATIVE), | ||
| }, | ||
| { | ||
| key: ModelType.PREDICTIVE, | ||
| label: formatModelTypeDisplay(ModelType.PREDICTIVE), | ||
| }, | ||
| ]; | ||
|
|
||
| type RegisterModelTypeFieldProps = { | ||
| modelCustomProperties: ModelRegistryCustomProperties | undefined; | ||
| onModelCustomPropertiesChange: (next: ModelRegistryCustomProperties) => void; | ||
| isRequired?: boolean; | ||
| }; | ||
|
|
||
| const RegisterModelTypeField: React.FC<RegisterModelTypeFieldProps> = ({ | ||
| modelCustomProperties, | ||
| onModelCustomPropertiesChange, | ||
| isRequired, | ||
| }) => { | ||
| const stored = getModelTypeStoredValueFromCustomProperties(modelCustomProperties); | ||
|
|
||
| const handleChange = (key: string, isPlaceholder: boolean) => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The handleChange callback checks both isPlaceholder and key === MODEL_TYPE_PLACEHOLDER_KEY. The isPlaceholder check alone should be sufficient — checking the key as well is redundant but harmless. |
||
| if (isPlaceholder) { | ||
| onModelCustomPropertiesChange( | ||
| buildCustomPropertiesWithModelType(modelCustomProperties, undefined), | ||
| ); | ||
| return; | ||
| } | ||
| if (key === ModelType.GENERATIVE || key === ModelType.PREDICTIVE) { | ||
| onModelCustomPropertiesChange(buildCustomPropertiesWithModelType(modelCustomProperties, key)); | ||
| } | ||
| }; | ||
|
|
||
| return ( | ||
| <FormGroup label="Model type" isRequired={isRequired} fieldId="register-model-type"> | ||
| <FormFieldset | ||
| field="Model type" | ||
| component={ | ||
| <SimpleSelect | ||
| options={MODEL_TYPE_SELECT_OPTIONS} | ||
| value={stored ?? undefined} | ||
| onChange={handleChange} | ||
| placeholder="Select model type" | ||
| isFullWidth | ||
| dataTestId="register-model-type-select" | ||
| previewDescription={false} | ||
| popperProps={{ direction: 'down' }} | ||
| toggleProps={{ id: 'register-model-type-toggle' }} | ||
| /> | ||
| } | ||
| /> | ||
| </FormGroup> | ||
| ); | ||
| }; | ||
|
|
||
| export default RegisterModelTypeField; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In here we can reuse reuse getModelTypeStoredValueFromCustomProperties from registerModelTypeUtils.ts , or even better - buildCustomPropertiesWithModelType
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reimplemented via
getModelTypeStoredValueFromCustomPropertiesandbuildCustomPropertiesWithModelType