From d56cb1bcc8c558d3a1bad416cbca559ea50728c7 Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Thu, 15 Jun 2023 23:04:04 -0400 Subject: [PATCH 01/42] Clean up --- azimuth/routers/config.py | 17 ++++++++--------- tests/test_routers/test_config.py | 6 +----- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/azimuth/routers/config.py b/azimuth/routers/config.py index 790ef6a9..08f1eab1 100644 --- a/azimuth/routers/config.py +++ b/azimuth/routers/config.py @@ -5,7 +5,6 @@ import structlog from fastapi import APIRouter, Body, Depends, HTTPException, Query -from pydantic import ValidationError from starlette.status import HTTP_400_BAD_REQUEST, HTTP_500_INTERNAL_SERVER_ERROR from azimuth.app import ( @@ -85,19 +84,21 @@ def patch_config( config: AzimuthConfig = Depends(get_config), partial_config: Dict = Body(...), ) -> AzimuthConfig: + log.info(f"Validating config change with {partial_config}.") + new_config = update_config(old_config=config, partial_config=partial_config) + if attribute_changed_in_config("artifact_path", partial_config, config): raise HTTPException( HTTP_400_BAD_REQUEST, detail="Cannot edit artifact_path, otherwise config history would become inconsistent.", ) + if new_config.large_dask_cluster != config.large_dask_cluster: + cluster = default_cluster(new_config.large_dask_cluster) + else: + cluster = task_manager.cluster + try: - log.info(f"Validating config change with {partial_config}.") - new_config = update_config(old_config=config, partial_config=partial_config) - if attribute_changed_in_config("large_dask_cluster", partial_config, config): - cluster = default_cluster(partial_config["large_dask_cluster"]) - else: - cluster = task_manager.cluster run_startup_tasks(new_config, cluster) log.info(f"Config successfully updated with {partial_config}.") except Exception as e: @@ -107,8 +108,6 @@ def patch_config( log.info("Config update cancelled.") if isinstance(e, AzimuthValidationError): raise HTTPException(HTTP_400_BAD_REQUEST, detail=str(e)) - if isinstance(e, ValidationError): - raise else: raise HTTPException( HTTP_500_INTERNAL_SERVER_ERROR, detail="Error when loading the new config." diff --git a/tests/test_routers/test_config.py b/tests/test_routers/test_config.py index 5c639cd0..3f258202 100644 --- a/tests/test_routers/test_config.py +++ b/tests/test_routers/test_config.py @@ -242,8 +242,6 @@ def test_get_config(app: FastAPI): def test_update_config(app: FastAPI, wait_for_startup_after): client = TestClient(app) initial_config = client.get("/config").json() - initial_contract = initial_config["model_contract"] - initial_pipelines = initial_config["pipelines"] initial_config_count = len(client.get("/config/history").json()) resp = client.patch( @@ -309,9 +307,7 @@ def test_update_config(app: FastAPI, wait_for_startup_after): assert not loaded_configs[-1]["config"]["pipelines"] # Revert config change - _ = client.patch( - "/config", json={"model_contract": initial_contract, "pipelines": initial_pipelines} - ) + _ = client.patch("/config", json=initial_config) loaded_configs = client.get("/config/history").json() assert loaded_configs[-1]["config"] == loaded_configs[initial_config_count - 1]["config"] From 9e40e507612a39d2eaf75635e7221f3963902a19 Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Thu, 15 Jun 2023 23:50:26 -0400 Subject: [PATCH 02/42] Fix updating config with equivalent `artifact_path` - Have Pydantic parse `artifact_path` as an absolute path. - Have only Pydantic deal with the `partial_config: Dict`, and check permission to update the config on the resulting updated config (instead of on the `partial_config: Dict`). - As a result, I can update from a relative `artifact_path="cache"` to an absolute `artifact_path="/Users/joseph/azimuth/cache"` for example, or even weird stuff like `artifact_path="/Users/joseph/azimuth/cache/../cache"`. --- azimuth/config.py | 6 +++++- azimuth/routers/config.py | 25 ++++++++++++++----------- tests/test_config.py | 8 ++++++++ tests/test_routers/test_config.py | 22 ++++++++++++++++++++-- 4 files changed, 47 insertions(+), 14 deletions(-) diff --git a/azimuth/config.py b/azimuth/config.py index bfee3133..a00f2eda 100644 --- a/azimuth/config.py +++ b/azimuth/config.py @@ -284,11 +284,15 @@ def get_project_hash(self): class ArtifactsConfig(AzimuthBaseSettings, extra=Extra.ignore): artifact_path: str = Field( - "cache", + default_factory=lambda: os.path.abspath("cache"), description="Where to store artifacts (Azimuth config history, HDF5 files, HF datasets).", exclude_from_cache=True, ) + @validator("artifact_path") + def validate_artifact_path(cls, artifact_path): + return os.path.abspath(artifact_path) + def get_config_history_path(self): return f"{self.artifact_path}/config_history.jsonl" diff --git a/azimuth/routers/config.py b/azimuth/routers/config.py index 08f1eab1..9740178e 100644 --- a/azimuth/routers/config.py +++ b/azimuth/routers/config.py @@ -1,11 +1,16 @@ # Copyright ServiceNow, Inc. 2021 – 2022 # This source code is licensed under the Apache 2.0 license found in the LICENSE file # in the root directory of this source tree. -from typing import Any, Dict, List +import os +from typing import Dict, List import structlog from fastapi import APIRouter, Body, Depends, HTTPException, Query -from starlette.status import HTTP_400_BAD_REQUEST, HTTP_500_INTERNAL_SERVER_ERROR +from starlette.status import ( + HTTP_400_BAD_REQUEST, + HTTP_403_FORBIDDEN, + HTTP_500_INTERNAL_SERVER_ERROR, +) from azimuth.app import ( get_config, @@ -87,11 +92,7 @@ def patch_config( log.info(f"Validating config change with {partial_config}.") new_config = update_config(old_config=config, partial_config=partial_config) - if attribute_changed_in_config("artifact_path", partial_config, config): - raise HTTPException( - HTTP_400_BAD_REQUEST, - detail="Cannot edit artifact_path, otherwise config history would become inconsistent.", - ) + assert_permission_to_update_config(old_config=config, new_config=new_config) if new_config.large_dask_cluster != config.large_dask_cluster: cluster = default_cluster(new_config.large_dask_cluster) @@ -116,7 +117,9 @@ def patch_config( return new_config -def attribute_changed_in_config( - attribute: str, partial_config: Dict[str, Any], config: AzimuthConfig -) -> bool: - return attribute in partial_config and partial_config[attribute] != getattr(config, attribute) +def assert_permission_to_update_config(*, old_config: AzimuthConfig, new_config: AzimuthConfig): + if old_config.artifact_path != new_config.artifact_path: + raise HTTPException( + HTTP_403_FORBIDDEN, + detail="Cannot edit artifact_path, otherwise config history would become inconsistent.", + ) diff --git a/tests/test_config.py b/tests/test_config.py index dff7757b..b8e18551 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -8,6 +8,7 @@ from pydantic import ValidationError from azimuth.config import ( + ArtifactsConfig, AzimuthConfig, AzimuthConfigHistoryWithHash, PipelineDefinition, @@ -289,3 +290,10 @@ def test_config_history_with_hash(): ValidationError, match="1 validation error for AzimuthConfigHistoryWithHash\nconfig -> name" ): AzimuthConfigHistoryWithHash(config={"name": None}) + + +def test_artifact_path_equality(): + # This is important since we forbid updating the config if the artifact_path differs. + default = ArtifactsConfig() + assert ArtifactsConfig(artifact_path="cache/../cache") == default + assert ArtifactsConfig(artifact_path=f"{os.getcwd()}/cache") == default diff --git a/tests/test_routers/test_config.py b/tests/test_routers/test_config.py index 3f258202..04607537 100644 --- a/tests/test_routers/test_config.py +++ b/tests/test_routers/test_config.py @@ -1,5 +1,12 @@ +import os.path + from fastapi import FastAPI -from starlette.status import HTTP_200_OK, HTTP_400_BAD_REQUEST, HTTP_500_INTERNAL_SERVER_ERROR +from starlette.status import ( + HTTP_200_OK, + HTTP_400_BAD_REQUEST, + HTTP_403_FORBIDDEN, + HTTP_500_INTERNAL_SERVER_ERROR, +) from starlette.testclient import TestClient from azimuth.config import SupportedLanguage, config_defaults_per_language @@ -23,7 +30,7 @@ def test_get_default_config(app: FastAPI): "persistent_id": "row_idx", }, "rejection_class": "REJECTION_CLASS", - "artifact_path": "cache", + "artifact_path": os.path.abspath("cache"), "batch_size": 32, "use_cuda": "auto", "large_dask_cluster": False, @@ -244,6 +251,17 @@ def test_update_config(app: FastAPI, wait_for_startup_after): initial_config = client.get("/config").json() initial_config_count = len(client.get("/config/history").json()) + resp = client.patch("/config", json={"artifact_path": "something/else"}) + assert resp.status_code == HTTP_403_FORBIDDEN, resp.text + + relative_artifact_path = os.path.relpath(initial_config["artifact_path"]) + assert relative_artifact_path != initial_config["artifact_path"] + resp = client.patch("/config", json={"artifact_path": relative_artifact_path}) + assert resp.status_code == HTTP_200_OK, resp.text + assert resp.json() == initial_config + new_config_count = len(client.get("/config/history").json()) + assert new_config_count == initial_config_count + resp = client.patch( "/config", json={"model_contract": "file_based_text_classification", "pipelines": None}, From 9019fd24d2decf4c835654b8624f3d67f6082503 Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Thu, 15 Jun 2023 22:19:24 -0400 Subject: [PATCH 03/42] Add a route to validate a config without applying it --- azimuth/routers/config.py | 19 +++++++ webapp/src/services/api.ts | 18 +++++-- webapp/src/types/generated/generatedTypes.ts | 56 ++++++++++++++++++++ 3 files changed, 89 insertions(+), 4 deletions(-) diff --git a/azimuth/routers/config.py b/azimuth/routers/config.py index 9740178e..26e72a29 100644 --- a/azimuth/routers/config.py +++ b/azimuth/routers/config.py @@ -77,6 +77,25 @@ def get_config_def( return config +@router.patch( + "/validate", + summary="Validate config", + description="Validate the given partial config update and return the complete config that would" + " result if this update was applied.", + response_model=AzimuthConfig, + dependencies=[Depends(require_editable_config)], +) +def validate_config( + config: AzimuthConfig = Depends(get_config), + partial_config: Dict = Body(...), +) -> AzimuthConfig: + new_config = update_config(old_config=config, partial_config=partial_config) + + assert_permission_to_update_config(old_config=config, new_config=new_config) + + return new_config + + @router.patch( "", summary="Update config", diff --git a/webapp/src/services/api.ts b/webapp/src/services/api.ts index 46443387..4972dc45 100644 --- a/webapp/src/services/api.ts +++ b/webapp/src/services/api.ts @@ -269,14 +269,23 @@ export const api = createApi({ providesTags: [{ type: "Config" }], queryFn: responseToData( fetchApi({ path: "/config", method: "get" }), - "Something went wrong fetching config" + "Something went wrong fetching the config" ), }), getDefaultConfig: build.query({ providesTags: [{ type: "DefaultConfig" }], queryFn: responseToData( fetchApi({ path: "/config/default", method: "get" }), - "Something went wrong fetching default config" + "Something went wrong fetching the default config" + ), + }), + validateConfig: build.mutation< + AzimuthConfig, + { jobId: string; body: Partial } + >({ + queryFn: responseToData( + fetchApi({ path: "/config/validate", method: "patch" }), + "Something went wrong validating the config" ), }), updateConfig: build.mutation< @@ -285,7 +294,7 @@ export const api = createApi({ >({ queryFn: responseToData( fetchApi({ path: "/config", method: "patch" }), - "Something went wrong updating config" + "Something went wrong updating the config" ), // We invalidate Status first, so StatusCheck stops rendering the app if // necessary. We await queryFulfilled before invalidating the other tags. @@ -353,7 +362,7 @@ export const api = createApi({ providesTags: () => [{ type: "Status" }], queryFn: responseToData( fetchApi({ path: "/status", method: "get" }), - "Something went wrong fetching status" + "Something went wrong fetching the status" ), }), }), @@ -380,6 +389,7 @@ export const { getStatus: getStatusEndpoint, getTopWords: getTopWordsEndpoint, getUtterances: getUtterancesEndpoint, + validateConfig: validateConfigEndpoint, updateConfig: updateConfigEndpoint, updateDataActions: updateDataActionsEndpoint, } = api.endpoints; diff --git a/webapp/src/types/generated/generatedTypes.ts b/webapp/src/types/generated/generatedTypes.ts index 6ff271b4..b78b6d17 100644 --- a/webapp/src/types/generated/generatedTypes.ts +++ b/webapp/src/types/generated/generatedTypes.ts @@ -34,6 +34,10 @@ export interface paths { /** Update the config. */ patch: operations["patch_config_config_patch"]; }; + "/config/validate": { + /** Validate the given partial config update and return the complete config that would result if this update was applied. */ + patch: operations["validate_config_config_validate_patch"]; + }; "/dataset_splits/{dataset_split_name}/class_overlap/plot": { /** Get a plot of class overlap using Spectral clustering and Monte-Carlo sampling (currently set to all samples). */ get: operations["get_class_overlap_plot_dataset_splits__dataset_split_name__class_overlap_plot_get"]; @@ -1309,6 +1313,58 @@ export interface operations { }; }; }; + /** Validate the given partial config update and return the complete config that would result if this update was applied. */ + validate_config_config_validate_patch: { + responses: { + /** Successful Response */ + 200: { + content: { + "application/json": components["schemas"]["AzimuthConfig"]; + }; + }; + /** Bad Request */ + 400: { + content: { + "application/json": components["schemas"]["HTTPExceptionModel"]; + }; + }; + /** Unauthorized */ + 401: { + content: { + "application/json": components["schemas"]["HTTPExceptionModel"]; + }; + }; + /** Forbidden */ + 403: { + content: { + "application/json": components["schemas"]["HTTPExceptionModel"]; + }; + }; + /** Not Found */ + 404: { + content: { + "application/json": components["schemas"]["HTTPExceptionModel"]; + }; + }; + /** Unprocessable Entity */ + 422: { + content: { + "application/json": components["schemas"]["HTTPExceptionModel"]; + }; + }; + /** Service Unavailable */ + 503: { + content: { + "application/json": components["schemas"]["HTTPExceptionModel"]; + }; + }; + }; + requestBody: { + content: { + "application/json": { [key: string]: any }; + }; + }; + }; /** Get a plot of class overlap using Spectral clustering and Monte-Carlo sampling (currently set to all samples). */ get_class_overlap_plot_dataset_splits__dataset_split_name__class_overlap_plot_get: { parameters: { From 42509e900e1dceaa4dcd891d0be72f5b2859303b Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Thu, 15 Jun 2023 14:09:35 -0400 Subject: [PATCH 04/42] Create reusable component `FileInputButton` --- .../components/Analysis/UtterancesTable.tsx | 68 ++++++++----------- webapp/src/components/FileInputButton.tsx | 28 ++++++++ 2 files changed, 56 insertions(+), 40 deletions(-) create mode 100644 webapp/src/components/FileInputButton.tsx diff --git a/webapp/src/components/Analysis/UtterancesTable.tsx b/webapp/src/components/Analysis/UtterancesTable.tsx index 96bd140f..0b782342 100644 --- a/webapp/src/components/Analysis/UtterancesTable.tsx +++ b/webapp/src/components/Analysis/UtterancesTable.tsx @@ -33,6 +33,7 @@ import CopyButton from "components/CopyButton"; import Description from "components/Description"; import OutcomeIcon from "components/Icons/OutcomeIcon"; import TargetIcon from "components/Icons/Target"; +import FileInputButton from "components/FileInputButton"; import Loading from "components/Loading"; import SmartTagFamilyBadge from "components/SmartTagFamilyBadge"; import { Column, RowProps, Table } from "components/Table"; @@ -436,35 +437,28 @@ const UtterancesTable: React.FC = ({ }, ]; - const importProposedActions = (file: File) => { - const fileReader = new FileReader(); - fileReader.onload = ({ target }) => { - if (target) { - const result = target.result as string; - const [header, ...rows] = result.trimEnd().split(/\r?\n/); - if (rows.length === 0) { - raiseErrorToast("There are no records in the CSV file."); - return; - } - if (header !== `${config.columns.persistent_id},proposed_action`) { - raiseErrorToast( - `The CSV file must have column headers ${config.columns.persistent_id} and proposed_action, in that order.` - ); - return; - } + const importProposedActions = (text: string) => { + const [header, ...rows] = text.trimEnd().split(/\r?\n/); + if (rows.length === 0) { + raiseErrorToast("There are no records in the CSV file."); + return; + } + if (header !== `${config.columns.persistent_id},proposed_action`) { + raiseErrorToast( + `The CSV file must have column headers ${config.columns.persistent_id} and proposed_action, in that order.` + ); + return; + } - const body = rows.map((row) => { - const [persistentId, dataAction] = row.split(","); - return { persistentId, dataAction } as UtterancePatch; - }); - updateDataAction({ - ignoreNotFound: true, - body, - ...getUtterancesQueryState, - }); - } - }; - fileReader.readAsText(file); + const body = rows.map((row) => { + const [persistentId, dataAction] = row.split(","); + return { persistentId, dataAction } as UtterancePatch; + }); + updateDataAction({ + ignoreNotFound: true, + body, + ...getUtterancesQueryState, + }); }; const RowLink = (props: RowProps) => ( @@ -484,19 +478,13 @@ const UtterancesTable: React.FC = ({ link="user-guide/exploration-space/utterance-table/" /> - + +); + +export default React.memo(FileInputButton); From bb26895760eab31a7c1ad6003d74d3ccdd0c9d01 Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Thu, 15 Jun 2023 14:22:39 -0400 Subject: [PATCH 05/42] Fix uploading same file twice --- webapp/src/components/FileInputButton.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/webapp/src/components/FileInputButton.tsx b/webapp/src/components/FileInputButton.tsx index f40c1bf2..74d2a4b4 100644 --- a/webapp/src/components/FileInputButton.tsx +++ b/webapp/src/components/FileInputButton.tsx @@ -20,6 +20,8 @@ const FileInputButton: React.FC< }; fileReader.readAsText(target.files[0]); } + // Reset input so that the same file can be selected again. + target.value = ""; }} /> From 9dcdc5c0710a8ce4eb95b666984b9eb9576fdd92 Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Fri, 16 Jun 2023 00:13:22 -0400 Subject: [PATCH 06/42] Log changes --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95aebdf7..92050da1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,5 +19,6 @@ Released changes are shown in the ### Removed ### Fixed +- Fixed importing the same proposed actions CSV file twice ### Security From 88bdec0634389e3c08c8e842706c4a74394eca29 Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Fri, 16 Jun 2023 16:57:19 -0400 Subject: [PATCH 07/42] Clarify comment --- webapp/src/components/FileInputButton.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapp/src/components/FileInputButton.tsx b/webapp/src/components/FileInputButton.tsx index 74d2a4b4..08542366 100644 --- a/webapp/src/components/FileInputButton.tsx +++ b/webapp/src/components/FileInputButton.tsx @@ -20,7 +20,7 @@ const FileInputButton: React.FC< }; fileReader.readAsText(target.files[0]); } - // Reset input so that the same file can be selected again. + // Reset input value so that selecting the same file re-triggers onChange. target.value = ""; }} /> From ec30cbfe5442a63f71c0b136d066b38e1aba5214 Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Fri, 16 Jun 2023 17:01:01 -0400 Subject: [PATCH 08/42] Even more --- webapp/src/components/FileInputButton.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapp/src/components/FileInputButton.tsx b/webapp/src/components/FileInputButton.tsx index 08542366..8c675c8a 100644 --- a/webapp/src/components/FileInputButton.tsx +++ b/webapp/src/components/FileInputButton.tsx @@ -20,7 +20,7 @@ const FileInputButton: React.FC< }; fileReader.readAsText(target.files[0]); } - // Reset input value so that selecting the same file re-triggers onChange. + // Reset input value so that selecting the same file again re-triggers onChange. target.value = ""; }} /> From a15eadd0241b897c013acd50ba8f3ca508f202a4 Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Thu, 15 Jun 2023 18:17:17 -0400 Subject: [PATCH 09/42] Create reusable conversion functions between `AzimuthConfig` and `ConfigState` --- webapp/src/pages/Settings.tsx | 37 ++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/webapp/src/pages/Settings.tsx b/webapp/src/pages/Settings.tsx index 0dd149e4..94d7fc16 100644 --- a/webapp/src/pages/Settings.tsx +++ b/webapp/src/pages/Settings.tsx @@ -55,6 +55,24 @@ type MetricState = MetricDefinition & { name: string }; type ConfigState = Omit & { metrics: MetricState[] }; +const azimuthConfigToConfigState = ({ + metrics, + ...rest +}: AzimuthConfig): ConfigState => ({ + ...rest, + metrics: Object.entries(metrics).map(([name, m]) => ({ name, ...m })), +}); + +const configStateToAzimuthConfig = ({ + metrics, + ...rest +}: Partial): Partial => ({ + ...rest, + ...(metrics && { + metrics: Object.fromEntries(metrics.map(({ name, ...m }) => [name, m])), + }), +}); + const CONFIG_UPDATE_MESSAGE = "Please wait while the config changes are validated."; const PERCENTAGE = { scale: 100, units: "%", inputProps: { min: 0, max: 100 } }; @@ -152,14 +170,10 @@ const Settings: React.FC = ({ open, onClose }) => { SupportedLanguage | undefined >(); const { data: azimuthConfig } = getConfigEndpoint.useQuery({ jobId }); - const config = React.useMemo(() => { - if (azimuthConfig === undefined) return undefined; - const { metrics, ...rest } = azimuthConfig; - return { - metrics: Object.entries(metrics).map(([name, m]) => ({ name, ...m })), - ...rest, - }; - }, [azimuthConfig]); + const config = React.useMemo( + () => azimuthConfig && azimuthConfigToConfigState(azimuthConfig), + [azimuthConfig] + ); const [updateConfig, { isLoading: isUpdatingConfig }] = updateConfigEndpoint.useMutation(); @@ -335,12 +349,7 @@ const Settings: React.FC = ({ open, onClose }) => { onClick={() => { updateConfig({ jobId, - body: { - ...partialConfig, - metrics: Object.fromEntries( - resultingConfig.metrics.map(({ name, ...m }) => [name, m]) - ), - }, + body: configStateToAzimuthConfig(partialConfig), }) .unwrap() .then( From 6347aaf97099c97348a1030e5b7270ad12a73df7 Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Fri, 16 Jun 2023 11:25:56 -0400 Subject: [PATCH 10/42] Add missing config fields that may trigger time-consuming computations --- webapp/src/pages/Settings.tsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/webapp/src/pages/Settings.tsx b/webapp/src/pages/Settings.tsx index 94d7fc16..337b3190 100644 --- a/webapp/src/pages/Settings.tsx +++ b/webapp/src/pages/Settings.tsx @@ -120,13 +120,19 @@ const USE_CUDA_OPTIONS = ["auto", "true", "false"] as const; type UseCUDAOption = typeof USE_CUDA_OPTIONS[number]; const FIELDS_TRIGGERING_STARTUP_TASKS: (keyof ConfigState)[] = [ + "dataset", + "columns", + "rejection_class", "behavioral_testing", "similarity", "dataset_warnings", "syntax", + "model_contract", "pipelines", "uncertainty", + "saliency_layer", "metrics", + "language", ]; type KnownPostprocessor = TemperatureScaling | ThresholdConfig; From 7d4a082a8e54cbfcefd6abcca911275b4d330d52 Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Fri, 16 Jun 2023 16:59:45 -0400 Subject: [PATCH 11/42] Fix download icon semantic by using the identical `Download` icon instead of `GetApp`. --- webapp/src/components/Analysis/UtterancesTable.tsx | 8 ++++---- .../PerturbationTestingExporter.tsx | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/webapp/src/components/Analysis/UtterancesTable.tsx b/webapp/src/components/Analysis/UtterancesTable.tsx index 0b782342..50f07768 100644 --- a/webapp/src/components/Analysis/UtterancesTable.tsx +++ b/webapp/src/components/Analysis/UtterancesTable.tsx @@ -1,13 +1,13 @@ import { ArrowDropDown, Close, + Download, Fullscreen, - GetApp, SvgIconComponent, + Upload, } from "@mui/icons-material"; import FilterAltOutlinedIcon from "@mui/icons-material/FilterAltOutlined"; import MultilineChartIcon from "@mui/icons-material/MultilineChart"; -import UploadIcon from "@mui/icons-material/Upload"; import { Box, Button, @@ -480,7 +480,7 @@ const UtterancesTable: React.FC = ({ } + startIcon={} onFileRead={importProposedActions} > Import @@ -490,7 +490,7 @@ const UtterancesTable: React.FC = ({ aria-controls="export-menu" aria-haspopup="true" onClick={(event) => setAnchorEl(event.currentTarget)} - startIcon={} + startIcon={} endIcon={} > Export diff --git a/webapp/src/components/PerturbationTestingSummary/PerturbationTestingExporter.tsx b/webapp/src/components/PerturbationTestingSummary/PerturbationTestingExporter.tsx index ad7b821b..ea1ffa18 100644 --- a/webapp/src/components/PerturbationTestingSummary/PerturbationTestingExporter.tsx +++ b/webapp/src/components/PerturbationTestingSummary/PerturbationTestingExporter.tsx @@ -1,4 +1,4 @@ -import { ArrowDropDown, GetApp } from "@mui/icons-material"; +import { ArrowDropDown, Download } from "@mui/icons-material"; import { Button, Menu, MenuItem } from "@mui/material"; import React from "react"; import { QueryPipelineState } from "types/models"; @@ -42,7 +42,7 @@ const PerturbationTestingExporter: React.FC = ({ jobId, pipeline }) => { aria-controls="perturbation-testing-exporter-menu" aria-haspopup="true" onClick={handleClick} - startIcon={} + startIcon={} endIcon={} > Export From e860569a80d523dd05b0914ca41765da22da5404 Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Thu, 15 Jun 2023 22:21:53 -0400 Subject: [PATCH 12/42] Add button to upload a config file --- webapp/src/pages/Settings.tsx | 125 +++++++++++++++++++++------------- 1 file changed, 78 insertions(+), 47 deletions(-) diff --git a/webapp/src/pages/Settings.tsx b/webapp/src/pages/Settings.tsx index 337b3190..4bb8cd06 100644 --- a/webapp/src/pages/Settings.tsx +++ b/webapp/src/pages/Settings.tsx @@ -1,4 +1,4 @@ -import { Close, Warning } from "@mui/icons-material"; +import { Close, Upload, Warning } from "@mui/icons-material"; import { Box, Button, @@ -22,6 +22,7 @@ import { } from "@mui/material"; import noData from "assets/void.svg"; import AccordionLayout from "components/AccordionLayout"; +import FileInputButton from "components/FileInputButton"; import Loading from "components/Loading"; import AutocompleteStringField from "components/Settings/AutocompleteStringField"; import CustomObjectFields from "components/Settings/CustomObjectFields"; @@ -37,6 +38,7 @@ import { getConfigEndpoint, getDefaultConfigEndpoint, updateConfigEndpoint, + validateConfigEndpoint, } from "services/api"; import { AzimuthConfig, @@ -50,6 +52,7 @@ import { } from "types/api"; import { PickByValue } from "types/models"; import { UNKNOWN_ERROR } from "utils/const"; +import { raiseErrorToast } from "utils/helpers"; type MetricState = MetricDefinition & { name: string }; @@ -181,9 +184,14 @@ const Settings: React.FC = ({ open, onClose }) => { [azimuthConfig] ); + const [validateConfig, { isLoading: isValidatingConfig }] = + validateConfigEndpoint.useMutation(); + const [updateConfig, { isLoading: isUpdatingConfig }] = updateConfigEndpoint.useMutation(); + const areInputsDisabled = isValidatingConfig || isUpdatingConfig; + const [partialConfig, setPartialConfig] = React.useState< Partial >({}); @@ -263,6 +271,22 @@ const Settings: React.FC = ({ open, onClose }) => { metricsNames.has("") || resultingConfig.metrics.some(({ class_name }) => class_name.trim() === ""); + const handleFileRead = (text: string) => { + try { + const body = JSON.parse(text); + validateConfig({ jobId, body }) + .unwrap() + .then((config) => setPartialConfig(azimuthConfigToConfigState(config))) + .catch(() => {}); // Avoid the uncaught error log. + } catch (error) { + raiseErrorToast( + `Something went wrong parsing JSON file\n${ + (error as SyntaxError).message + }` + ); + } + }; + const renderDialog = (children: React.ReactNode) => ( = ({ open, onClose }) => { open={open} > - - - View and edit certain fields from your config file. Once your - changes are saved, expect some delays for recomputing the affected - tasks. + + + Configuration + } + onFileRead={handleFileRead} + > + Import JSON config file + { if ( isEmptyPartialConfig || @@ -319,7 +349,7 @@ const Settings: React.FC = ({ open, onClose }) => { Date: Sat, 17 Jun 2023 00:58:37 -0400 Subject: [PATCH 14/42] Log changes --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92050da1..8af66ff0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Released changes are shown in the ## [Not released] ### Added +- Two buttons in the config UI to import/export a JSON config file. ### Changed From 6eca195212a5090a8a6d929694c9d884aee55c04 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 19 Jun 2023 10:19:24 -0400 Subject: [PATCH 15/42] Bump requests from 2.28.1 to 2.31.0 (#567) Bumps [requests](https://github.com/psf/requests) from 2.28.1 to 2.31.0. - [Release notes](https://github.com/psf/requests/releases) - [Changelog](https://github.com/psf/requests/blob/main/HISTORY.md) - [Commits](https://github.com/psf/requests/compare/v2.28.1...v2.31.0) --- updated-dependencies: - dependency-name: requests dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- poetry.lock | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/poetry.lock b/poetry.lock index 39b3e409..e71a275d 100644 --- a/poetry.lock +++ b/poetry.lock @@ -4439,21 +4439,21 @@ files = [ [[package]] name = "requests" -version = "2.28.1" +version = "2.31.0" description = "Python HTTP for Humans." category = "main" optional = false -python-versions = ">=3.7, <4" +python-versions = ">=3.7" files = [ - {file = "requests-2.28.1-py3-none-any.whl", hash = "sha256:8fefa2a1a1365bf5520aac41836fbee479da67864514bdb821f31ce07ce65349"}, - {file = "requests-2.28.1.tar.gz", hash = "sha256:7c5599b102feddaa661c826c56ab4fee28bfd17f5abca1ebbe3e7f19d7c97983"}, + {file = "requests-2.31.0-py3-none-any.whl", hash = "sha256:58cd2187c01e70e6e26505bca751777aa9f2ee0b7f4300988b709f44e013003f"}, + {file = "requests-2.31.0.tar.gz", hash = "sha256:942c5a758f98d790eaed1a29cb6eefc7ffb0d1cf7af05c3d2791656dbd6ad1e1"}, ] [package.dependencies] certifi = ">=2017.4.17" -charset-normalizer = ">=2,<3" +charset-normalizer = ">=2,<4" idna = ">=2.5,<4" -urllib3 = ">=1.21.1,<1.27" +urllib3 = ">=1.21.1,<3" [package.extras] socks = ["PySocks (>=1.5.6,!=1.5.7)"] From 2be51bad85e94f62b4c570411c08664be46379dd Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 19 Jun 2023 10:20:04 -0400 Subject: [PATCH 16/42] Bump mpmath from 1.2.1 to 1.3.0 (#566) Bumps [mpmath](https://github.com/fredrik-johansson/mpmath) from 1.2.1 to 1.3.0. - [Release notes](https://github.com/fredrik-johansson/mpmath/releases) - [Changelog](https://github.com/mpmath/mpmath/blob/master/CHANGES) - [Commits](https://github.com/fredrik-johansson/mpmath/compare/1.2.1...1.3.0) --- updated-dependencies: - dependency-name: mpmath dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- poetry.lock | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/poetry.lock b/poetry.lock index e71a275d..2ab9c535 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2483,18 +2483,20 @@ files = [ [[package]] name = "mpmath" -version = "1.2.1" +version = "1.3.0" description = "Python library for arbitrary-precision floating-point arithmetic" category = "main" optional = false python-versions = "*" files = [ - {file = "mpmath-1.2.1-py3-none-any.whl", hash = "sha256:604bc21bd22d2322a177c73bdb573994ef76e62edd595d17e00aff24b0667e5c"}, - {file = "mpmath-1.2.1.tar.gz", hash = "sha256:79ffb45cf9f4b101a807595bcb3e72e0396202e0b1d25d689134b48c4216a81a"}, + {file = "mpmath-1.3.0-py3-none-any.whl", hash = "sha256:a0b2b9fe80bbcd81a6647ff13108738cfb482d481d826cc0e02f5b35e5c88d2c"}, + {file = "mpmath-1.3.0.tar.gz", hash = "sha256:7a28eb2a9774d00c7bc92411c19a89209d5da7c4c9a9e227be8330a23a25b91f"}, ] [package.extras] develop = ["codecov", "pycodestyle", "pytest (>=4.6)", "pytest-cov", "wheel"] +docs = ["sphinx"] +gmpy = ["gmpy2 (>=2.1.0a4)"] tests = ["pytest (>=4.6)"] [[package]] From c0234d78d3ce02085509f0a751b54a89edc4f437 Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Mon, 19 Jun 2023 12:00:27 -0400 Subject: [PATCH 17/42] Add comment about toast --- webapp/src/pages/Settings.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/webapp/src/pages/Settings.tsx b/webapp/src/pages/Settings.tsx index 53489689..fd2de9f7 100644 --- a/webapp/src/pages/Settings.tsx +++ b/webapp/src/pages/Settings.tsx @@ -278,7 +278,7 @@ const Settings: React.FC = ({ open, onClose }) => { validateConfig({ jobId, body }) .unwrap() .then((config) => setPartialConfig(azimuthConfigToConfigState(config))) - .catch(() => {}); // Avoid the uncaught error log. + .catch(() => {}); // Avoid the uncaught error log. Toast already raised by `rtkQueryErrorInterceptor` middleware. } catch (error) { raiseErrorToast( `Something went wrong parsing JSON file\n${ @@ -404,7 +404,7 @@ const Settings: React.FC = ({ open, onClose }) => { }) .unwrap() .then(handleClose) - .catch(() => {}); // Avoid the uncaught error log. + .catch(() => {}); // Avoid the uncaught error log. Toast already raised by `rtkQueryErrorInterceptor` middleware. }} > Apply and close From 299c9ebde623b917c3be7585ae89f6b61a294910 Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Tue, 20 Jun 2023 11:30:12 -0400 Subject: [PATCH 18/42] Remove unnecessary box --- webapp/src/pages/Settings.tsx | 84 +++++++++++++++++------------------ 1 file changed, 41 insertions(+), 43 deletions(-) diff --git a/webapp/src/pages/Settings.tsx b/webapp/src/pages/Settings.tsx index fd2de9f7..9f00df6f 100644 --- a/webapp/src/pages/Settings.tsx +++ b/webapp/src/pages/Settings.tsx @@ -930,49 +930,47 @@ const Settings: React.FC = ({ open, onClose }) => { ); const getCommonFieldsConfigSection = () => ( - - - - - updatePartialConfig({ batch_size })} - {...INT} - /> - - updatePartialConfig({ - use_cuda: use_cuda === "auto" ? "auto" : use_cuda === "true", - }) - } - /> - - updatePartialConfig({ large_dask_cluster }) - } - /> - } - label="large_dask_cluster" - /> - - - + + + + updatePartialConfig({ batch_size })} + {...INT} + /> + + updatePartialConfig({ + use_cuda: use_cuda === "auto" ? "auto" : use_cuda === "true", + }) + } + /> + + updatePartialConfig({ large_dask_cluster }) + } + /> + } + label="large_dask_cluster" + /> + + ); const displayAnalysesCustomizationGeneralSection = () => ( From ab1956dd62ee2a2486e20640c1f88edce93e59f6 Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Tue, 20 Jun 2023 11:33:25 -0400 Subject: [PATCH 19/42] Remove unnecessary functions --- webapp/src/pages/Settings.tsx | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/webapp/src/pages/Settings.tsx b/webapp/src/pages/Settings.tsx index 9f00df6f..57760dd9 100644 --- a/webapp/src/pages/Settings.tsx +++ b/webapp/src/pages/Settings.tsx @@ -517,7 +517,7 @@ const Settings: React.FC = ({ open, onClose }) => { /> ); - const getProjectConfigSection = () => ( + const projectConfigSection = ( <> {displaySectionTitle("General")} @@ -573,7 +573,7 @@ const Settings: React.FC = ({ open, onClose }) => { ); - const getModelContractConfigSection = () => ( + const modelContractConfigSection = ( <> {displaySectionTitle("General")} @@ -929,7 +929,7 @@ const Settings: React.FC = ({ open, onClose }) => { ); - const getCommonFieldsConfigSection = () => ( + const commonFieldsConfigSection = ( = ({ open, onClose }) => { ); - const displayAnalysesCustomizationGeneralSection = () => ( + const analysesCustomizationGeneralSection = ( = ({ open, onClose }) => { ); - const getAnalysesCustomizationSection = () => ( + const analysesCustomizationSection = ( <> {displaySectionTitle("General")} - {displayAnalysesCustomizationGeneralSection()} + {analysesCustomizationGeneralSection} {displaySectionTitle("Dataset Warnings")} {getAnalysesCustomization("dataset_warnings")} {displaySectionTitle("Syntax")} @@ -1018,28 +1018,28 @@ const Settings: React.FC = ({ open, onClose }) => { link="reference/configuration/project/" defaultExpanded > - {getProjectConfigSection()} + {projectConfigSection} - {getModelContractConfigSection()} + {modelContractConfigSection} - {getCommonFieldsConfigSection()} + {commonFieldsConfigSection} - {getAnalysesCustomizationSection()} + {analysesCustomizationSection} ); From fc532bc0fad7badb1155bb792ea2bae2b44d8717 Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Tue, 20 Jun 2023 13:32:02 -0400 Subject: [PATCH 20/42] Customize form helper text font weight at the theme level --- webapp/src/components/Settings/StringArrayField.tsx | 1 - webapp/src/styles/theme.tsx | 9 ++++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/webapp/src/components/Settings/StringArrayField.tsx b/webapp/src/components/Settings/StringArrayField.tsx index 3337825c..b5b4d129 100644 --- a/webapp/src/components/Settings/StringArrayField.tsx +++ b/webapp/src/components/Settings/StringArrayField.tsx @@ -24,7 +24,6 @@ const StringArrayField: React.FC< {...params} {...FIELD_COMMON_PROPS} label={label} - FormHelperTextProps={{ sx: { fontWeight: "unset" } }} helperText={ <> Write a{/^[aeiou]/.test(units) && "n"} {units} and press enter diff --git a/webapp/src/styles/theme.tsx b/webapp/src/styles/theme.tsx index aca3cad0..a32a8ccf 100644 --- a/webapp/src/styles/theme.tsx +++ b/webapp/src/styles/theme.tsx @@ -1,5 +1,5 @@ import { Theme } from "@emotion/react"; -import { createTheme } from "@mui/material"; +import { createTheme, formHelperTextClasses } from "@mui/material"; import withStyles from "@mui/styles/withStyles"; import "styles/typography/gilroy/gilroy.css"; @@ -168,6 +168,13 @@ const customTheme: Theme = createTheme({ }, }, }, + MuiFormHelperText: { + styleOverrides: { + root: { + [`&:not(.${formHelperTextClasses.error})`]: { fontWeight: "unset" }, + }, + }, + }, }, }); From 4ceb69df9c59b91cfc0c49f0b84a1703367484d3 Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Tue, 20 Jun 2023 13:35:25 -0400 Subject: [PATCH 21/42] Fix checkbox hit box that was too big --- webapp/src/pages/Settings.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/webapp/src/pages/Settings.tsx b/webapp/src/pages/Settings.tsx index 57760dd9..d2ddad11 100644 --- a/webapp/src/pages/Settings.tsx +++ b/webapp/src/pages/Settings.tsx @@ -968,6 +968,7 @@ const Settings: React.FC = ({ open, onClose }) => { /> } label="large_dask_cluster" + sx={{ justifySelf: "start" }} // Make hit box tight on the component. /> From 2b967b06b8231497d0eab5728e72941e1fe5251a Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Tue, 20 Jun 2023 15:11:14 -0400 Subject: [PATCH 22/42] Add asterisk to required fields --- webapp/src/components/Settings/StringField.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/webapp/src/components/Settings/StringField.tsx b/webapp/src/components/Settings/StringField.tsx index d5078155..f37046ba 100644 --- a/webapp/src/components/Settings/StringField.tsx +++ b/webapp/src/components/Settings/StringField.tsx @@ -20,6 +20,7 @@ const StringField = ({ select={Boolean(options)} inputProps={{ sx: { textOverflow: "ellipsis" } }} value={value ?? ""} + required={!nullable && !options} {...(value?.trim() === "" && { error: true, helperText: "Set a value" })} onChange={ onChange && From 99fa008d5bbc5ddb8aaae3da9960c229dac26186 Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Tue, 20 Jun 2023 21:25:05 -0400 Subject: [PATCH 23/42] Fix alphabetical order in some imports --- .../src/components/ConfidenceHistogram/BinThresholdMarker.tsx | 2 +- webapp/src/components/Metrics/DeltaComputationBar.tsx | 2 +- webapp/src/components/NoMaxWidthTooltip.tsx | 2 +- webapp/src/components/Settings/StringArrayField.tsx | 2 +- webapp/src/components/ThresholdPlot.tsx | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/webapp/src/components/ConfidenceHistogram/BinThresholdMarker.tsx b/webapp/src/components/ConfidenceHistogram/BinThresholdMarker.tsx index 565105bd..0f680a18 100644 --- a/webapp/src/components/ConfidenceHistogram/BinThresholdMarker.tsx +++ b/webapp/src/components/ConfidenceHistogram/BinThresholdMarker.tsx @@ -1,4 +1,4 @@ -import { Typography, alpha, Box } from "@mui/material"; +import { Box, Typography, alpha } from "@mui/material"; type Props = { threshold: number; diff --git a/webapp/src/components/Metrics/DeltaComputationBar.tsx b/webapp/src/components/Metrics/DeltaComputationBar.tsx index 1afd69f9..5ebbff30 100644 --- a/webapp/src/components/Metrics/DeltaComputationBar.tsx +++ b/webapp/src/components/Metrics/DeltaComputationBar.tsx @@ -1,5 +1,5 @@ import React from "react"; -import { alpha, Box, Typography } from "@mui/material"; +import { Box, Typography, alpha } from "@mui/material"; import { motion } from "framer-motion"; type Props = { diff --git a/webapp/src/components/NoMaxWidthTooltip.tsx b/webapp/src/components/NoMaxWidthTooltip.tsx index 7a9d0dbc..2c969721 100644 --- a/webapp/src/components/NoMaxWidthTooltip.tsx +++ b/webapp/src/components/NoMaxWidthTooltip.tsx @@ -1,4 +1,4 @@ -import { TooltipProps, Tooltip, tooltipClasses, styled } from "@mui/material"; +import { Tooltip, TooltipProps, styled, tooltipClasses } from "@mui/material"; // From https://mui.com/material-ui/react-tooltip/#variable-width const NoMaxWidthTooltip = styled(({ className, ...props }: TooltipProps) => ( diff --git a/webapp/src/components/Settings/StringArrayField.tsx b/webapp/src/components/Settings/StringArrayField.tsx index b5b4d129..384a2f2f 100644 --- a/webapp/src/components/Settings/StringArrayField.tsx +++ b/webapp/src/components/Settings/StringArrayField.tsx @@ -1,7 +1,7 @@ import { Autocomplete, - TextField, Chip, + TextField, autocompleteClasses, chipClasses, } from "@mui/material"; diff --git a/webapp/src/components/ThresholdPlot.tsx b/webapp/src/components/ThresholdPlot.tsx index 8b250ef6..ae8003d1 100644 --- a/webapp/src/components/ThresholdPlot.tsx +++ b/webapp/src/components/ThresholdPlot.tsx @@ -1,10 +1,10 @@ import { Info } from "@mui/icons-material"; import { - alpha, Box, CircularProgress, Tooltip, Typography, + alpha, } from "@mui/material"; import makeStyles from "@mui/styles/makeStyles"; import React from "react"; From 102549aa03cbd04e035f2506d190a9850b264f2b Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Tue, 20 Jun 2023 23:08:07 -0400 Subject: [PATCH 24/42] Support any of the `TextFieldProps` in `StringArrayField` instead of supporting explicitly the `disabled` prop. --- .../src/components/Settings/StringArrayField.tsx | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/webapp/src/components/Settings/StringArrayField.tsx b/webapp/src/components/Settings/StringArrayField.tsx index 384a2f2f..28be6883 100644 --- a/webapp/src/components/Settings/StringArrayField.tsx +++ b/webapp/src/components/Settings/StringArrayField.tsx @@ -2,6 +2,7 @@ import { Autocomplete, Chip, TextField, + TextFieldProps, autocompleteClasses, chipClasses, } from "@mui/material"; @@ -9,9 +10,17 @@ import React from "react"; import { FieldProps, FIELD_COMMON_PROPS } from "./utils"; const StringArrayField: React.FC< - FieldProps & { label?: string; units?: string; disabled: boolean } -> = ({ value, onChange, label, units = label || "token", disabled }) => ( - & + FieldProps & { label?: string; units?: string } +> = ({ + value, + onChange, + label, + units = label || "token", + disabled, + ...props +}) => ( + disableClearable freeSolo multiple @@ -29,6 +38,7 @@ const StringArrayField: React.FC< Write a{/^[aeiou]/.test(units) && "n"} {units} and press enter } + {...props} /> )} renderTags={(value, getTagProps) => From a4711dac6e9311d1dc6e3346c81bc1ef4a9ab569 Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Wed, 21 Jun 2023 14:58:16 -0400 Subject: [PATCH 25/42] Use "a" in front of words starting with "u" since most nouns starting with "u" seems to make the "you" sound, like "user" or "union". --- webapp/src/components/Settings/StringArrayField.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapp/src/components/Settings/StringArrayField.tsx b/webapp/src/components/Settings/StringArrayField.tsx index 28be6883..d43229ba 100644 --- a/webapp/src/components/Settings/StringArrayField.tsx +++ b/webapp/src/components/Settings/StringArrayField.tsx @@ -35,7 +35,7 @@ const StringArrayField: React.FC< label={label} helperText={ <> - Write a{/^[aeiou]/.test(units) && "n"} {units} and press enter + Write a{/^[aeio]/.test(units) && "n"} {units} and press enter } {...props} From 315456bda8eb3d4b896e96ee7aac235db1d66e49 Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Mon, 26 Jun 2023 19:04:30 -0400 Subject: [PATCH 26/42] Add menu to load a previous config --- CHANGELOG.md | 1 + webapp/src/components/HashChip.tsx | 16 ++++++++++ webapp/src/pages/Settings.tsx | 48 +++++++++++++++++++++++++++++- webapp/src/services/api.ts | 9 ++++++ webapp/src/utils/format.ts | 6 ++++ 5 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 webapp/src/components/HashChip.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 8af66ff0..7989fe6d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Released changes are shown in the ### Added - Two buttons in the config UI to import/export a JSON config file. +- Drop-down menu to load a previous config. ### Changed diff --git a/webapp/src/components/HashChip.tsx b/webapp/src/components/HashChip.tsx new file mode 100644 index 00000000..1470f535 --- /dev/null +++ b/webapp/src/components/HashChip.tsx @@ -0,0 +1,16 @@ +import { Chip } from "@mui/material"; +import React from "react"; + +const HashChip: React.FC<{ hash: string }> = ({ hash }) => ( + theme.palette.getContrastText(`#${hash}`), + }} + /> +); + +export default React.memo(HashChip); diff --git a/webapp/src/pages/Settings.tsx b/webapp/src/pages/Settings.tsx index d2ddad11..5175d669 100644 --- a/webapp/src/pages/Settings.tsx +++ b/webapp/src/pages/Settings.tsx @@ -1,4 +1,4 @@ -import { Close, Download, Upload, Warning } from "@mui/icons-material"; +import { Close, Download, Restore, Upload, Warning } from "@mui/icons-material"; import { Box, Button, @@ -18,11 +18,14 @@ import { InputBaseComponentProps, inputClasses, inputLabelClasses, + Menu, + MenuItem, Typography, } from "@mui/material"; import noData from "assets/void.svg"; import AccordionLayout from "components/AccordionLayout"; import FileInputButton from "components/FileInputButton"; +import HashChip from "components/HashChip"; import Loading from "components/Loading"; import AutocompleteStringField from "components/Settings/AutocompleteStringField"; import CustomObjectFields from "components/Settings/CustomObjectFields"; @@ -36,6 +39,7 @@ import React from "react"; import { useParams } from "react-router-dom"; import { getConfigEndpoint, + getConfigHistoryEndpoint, getDefaultConfigEndpoint, updateConfigEndpoint, validateConfigEndpoint, @@ -53,6 +57,7 @@ import { import { PickByValue } from "types/models"; import { downloadBlob } from "utils/api"; import { UNKNOWN_ERROR } from "utils/const"; +import { formatDateISO } from "utils/format"; import { raiseErrorToast } from "utils/helpers"; type MetricState = MetricDefinition & { name: string }; @@ -223,6 +228,11 @@ const Settings: React.FC = ({ open, onClose }) => { language: language ?? resultingConfig.language, }); + const { data: configHistory } = getConfigHistoryEndpoint.useQuery({ jobId }); + + const [configHistoryAnchor, setConfigHistoryAnchor] = + React.useState(null); + const updatePartialConfig = React.useCallback( (update: Partial) => setPartialConfig((partialConfig) => ({ ...partialConfig, ...update })), @@ -307,6 +317,42 @@ const Settings: React.FC = ({ open, onClose }) => { Configuration + {configHistory?.length && ( + <> + + setConfigHistoryAnchor(null)} + > + {configHistory.map(({ config, created_on, hash }, index) => ( + { + setConfigHistoryAnchor(null); + setPartialConfig(azimuthConfigToConfigState(config)); + }} + > + {config.name} + + + {formatDateISO(new Date(created_on))} + + + ))} + + + )} `${formatNumberAsString(100 * value, digits)}%`; +const pad = (n: number) => String(n).padStart(2, "0"); + +export const formatDateISO = (date: Date) => + `${date.getFullYear()}-${pad(date.getMonth() + 1)}-${pad(date.getDate())} ` + + `${pad(date.getHours())}:${pad(date.getMinutes())}:${pad(date.getSeconds())}`; + export const camelToTitleCase = (camelCase: string) => // Safari and iOS browsers don't support lookbehind in regular expressions. camelCase.replace(/([a-z])(?=[A-Z0-9])|([A-Z0-9])(?=[A-Z][a-z])/g, "$& "); From 5069fc787c581bcaed65ff340c8efdf9c8d5428e Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Mon, 26 Jun 2023 20:24:57 -0400 Subject: [PATCH 27/42] Show 0, 3, or 6 chars of hash, based on collisions * Don't show the hash (hashSize = null) if no two different configs have the same name. * Show a 6-char hash if there is a collision in the first 3 chars. * Otherwise, show a 3-char hash. --- webapp/src/pages/Settings.tsx | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/webapp/src/pages/Settings.tsx b/webapp/src/pages/Settings.tsx index 5175d669..d786cd58 100644 --- a/webapp/src/pages/Settings.tsx +++ b/webapp/src/pages/Settings.tsx @@ -282,6 +282,24 @@ const Settings: React.FC = ({ open, onClose }) => { metricsNames.has("") || resultingConfig.metrics.some(({ class_name }) => class_name.trim() === ""); + const fullHashCount = new Set(configHistory?.map(({ hash }) => hash)).size; + const hashCount = new Set(configHistory?.map(({ hash }) => hash.slice(0, 3))) + .size; + const nameCount = new Set(configHistory?.map(({ config }) => config.name)) + .size; + + // Don't show the hash (hashSize = null) if no two different configs have the same name. + // Show a 6-char hash if there is a collision in the first 3 chars. + // Otherwise, show a 3-char hash. + // Probability of a hash collision with the 3-char hash: + // 10 different configs: 1 % + // 30 different configs: 10 % + // 76 different configs: 50 % + // With the 6-char hash: + // 581 different configs: 1 % + const hashChars = + nameCount === fullHashCount ? null : hashCount === fullHashCount ? 3 : 6; + const handleFileRead = (text: string) => { try { const body = JSON.parse(text); @@ -341,7 +359,7 @@ const Settings: React.FC = ({ open, onClose }) => { }} > {config.name} - + {hashChars && } Date: Mon, 26 Jun 2023 20:28:12 -0400 Subject: [PATCH 28/42] Clean up importing routers (#585) removing 13 places where we needed `as`. --- azimuth/app.py | 64 ++++++++++++++++++++------------------- azimuth/routers/config.py | 1 - 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/azimuth/app.py b/azimuth/app.py index 2b69fc58..738fcb19 100644 --- a/azimuth/app.py +++ b/azimuth/app.py @@ -155,7 +155,7 @@ def create_app() -> FastAPI: Returns: FastAPI. """ - app = FastAPI( + api = FastAPI( title="Azimuth API", description="Azimuth API", version="1.0", @@ -171,101 +171,103 @@ def create_app() -> FastAPI: ) # Setup routes - from azimuth.routers.app import router as app_router - from azimuth.routers.class_overlap import router as class_overlap_router - from azimuth.routers.config import router as config_router - from azimuth.routers.custom_utterances import router as custom_utterances_router - from azimuth.routers.dataset_warnings import router as dataset_warnings_router - from azimuth.routers.export import router as export_router - from azimuth.routers.model_performance.confidence_histogram import ( - router as confidence_histogram_router, + from azimuth.routers import ( + app, + class_overlap, + config, + custom_utterances, + dataset_warnings, + export, + top_words, + utterances, + ) + from azimuth.routers.model_performance import ( + confidence_histogram, + confusion_matrix, + metrics, + outcome_count, + utterance_count, ) - from azimuth.routers.model_performance.confusion_matrix import router as confusion_matrix_router - from azimuth.routers.model_performance.metrics import router as metrics_router - from azimuth.routers.model_performance.outcome_count import router as outcome_count_router - from azimuth.routers.model_performance.utterance_count import router as utterance_count_router - from azimuth.routers.top_words import router as top_words_router - from azimuth.routers.utterances import router as utterances_router from azimuth.utils.routers import require_application_ready, require_available_model api_router = APIRouter() - api_router.include_router(app_router, prefix="", tags=["App"]) - api_router.include_router(config_router, prefix="/config", tags=["Config"]) + api_router.include_router(app.router, prefix="", tags=["App"]) + api_router.include_router(config.router, prefix="/config", tags=["Config"]) api_router.include_router( - class_overlap_router, + class_overlap.router, prefix="/dataset_splits/{dataset_split_name}/class_overlap", tags=["Class Overlap"], dependencies=[Depends(require_application_ready)], ) api_router.include_router( - confidence_histogram_router, + confidence_histogram.router, prefix="/dataset_splits/{dataset_split_name}/confidence_histogram", tags=["Confidence Histogram"], dependencies=[Depends(require_application_ready), Depends(require_available_model)], ) api_router.include_router( - dataset_warnings_router, + dataset_warnings.router, prefix="/dataset_warnings", tags=["Dataset Warnings"], dependencies=[Depends(require_application_ready)], ) api_router.include_router( - metrics_router, + metrics.router, prefix="/dataset_splits/{dataset_split_name}/metrics", tags=["Metrics"], dependencies=[Depends(require_application_ready), Depends(require_available_model)], ) api_router.include_router( - outcome_count_router, + outcome_count.router, prefix="/dataset_splits/{dataset_split_name}/outcome_count", tags=["Outcome Count"], dependencies=[Depends(require_application_ready), Depends(require_available_model)], ) api_router.include_router( - utterance_count_router, + utterance_count.router, prefix="/dataset_splits/{dataset_split_name}/utterance_count", tags=["Utterance Count"], dependencies=[Depends(require_application_ready)], ) api_router.include_router( - utterances_router, + utterances.router, prefix="/dataset_splits/{dataset_split_name}/utterances", tags=["Utterances"], dependencies=[Depends(require_application_ready)], ) api_router.include_router( - export_router, + export.router, prefix="/export", tags=["Export"], dependencies=[Depends(require_application_ready)], ) api_router.include_router( - custom_utterances_router, + custom_utterances.router, prefix="/custom_utterances", tags=["Custom Utterances"], dependencies=[Depends(require_application_ready)], ) api_router.include_router( - top_words_router, + top_words.router, prefix="/dataset_splits/{dataset_split_name}/top_words", tags=["Top Words"], dependencies=[Depends(require_application_ready), Depends(require_available_model)], ) api_router.include_router( - confusion_matrix_router, + confusion_matrix.router, prefix="/dataset_splits/{dataset_split_name}/confusion_matrix", tags=["Confusion Matrix"], dependencies=[Depends(require_application_ready), Depends(require_available_model)], ) - app.include_router(api_router) + api.include_router(api_router) - app.add_middleware( + api.add_middleware( CORSMiddleware, allow_methods=["*"], allow_headers=["*"], ) - return app + return api def load_dataset_split_managers_from_config( diff --git a/azimuth/routers/config.py b/azimuth/routers/config.py index 26e72a29..33153f67 100644 --- a/azimuth/routers/config.py +++ b/azimuth/routers/config.py @@ -1,7 +1,6 @@ # Copyright ServiceNow, Inc. 2021 – 2022 # This source code is licensed under the Apache 2.0 license found in the LICENSE file # in the root directory of this source tree. -import os from typing import Dict, List import structlog From 2f75f3f2f32dd76e91387eb9b8864606cf50c4a0 Mon Sep 17 00:00:00 2001 From: "joseph.marinier" Date: Tue, 27 Jun 2023 09:40:05 -0400 Subject: [PATCH 29/42] Change icon same visual, better semantics --- webapp/src/pages/Settings.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/webapp/src/pages/Settings.tsx b/webapp/src/pages/Settings.tsx index d786cd58..6f128ef6 100644 --- a/webapp/src/pages/Settings.tsx +++ b/webapp/src/pages/Settings.tsx @@ -1,4 +1,4 @@ -import { Close, Download, Restore, Upload, Warning } from "@mui/icons-material"; +import { Close, Download, History, Upload, Warning } from "@mui/icons-material"; import { Box, Button, @@ -339,7 +339,7 @@ const Settings: React.FC = ({ open, onClose }) => { <>