From 71ada285e714f670f0b3dd409989afcf4ed4cd1e Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Fri, 15 Sep 2023 17:45:54 +0900 Subject: [PATCH] fix by review --- optuna_dashboard/_preference_setting.py | 2 +- optuna_dashboard/_serializer.py | 5 ++- optuna_dashboard/ts/action.ts | 26 +++++++----- optuna_dashboard/ts/apiClient.ts | 33 +++------------ .../ts/components/PreferentialTrials.tsx | 42 +++++++++++++------ 5 files changed, 56 insertions(+), 52 deletions(-) diff --git a/optuna_dashboard/_preference_setting.py b/optuna_dashboard/_preference_setting.py index 907fc2be..9f1e8dec 100644 --- a/optuna_dashboard/_preference_setting.py +++ b/optuna_dashboard/_preference_setting.py @@ -22,7 +22,7 @@ def _register_preference_feedback_component( component_type: OUTPUT_COMPONENT_TYPE, artifact_key: str | None = None, ) -> None: - value: dict[str, Any] = {"type": component_type} + value: dict[str, Any] = {"output_type": component_type} if artifact_key is not None: value["artifact_key"] = artifact_key storage.set_study_system_attr( diff --git a/optuna_dashboard/_serializer.py b/optuna_dashboard/_serializer.py index 5829f35d..016b7fb8 100644 --- a/optuna_dashboard/_serializer.py +++ b/optuna_dashboard/_serializer.py @@ -167,7 +167,10 @@ def serialize_study_detail( serialized["form_widgets"] = form_widgets if serialized["is_preferential"]: serialized["feedback_component_type"] = system_attrs.get( - _SYSTEM_ATTR_FEEDBACK_COMPONENT, {} + _SYSTEM_ATTR_FEEDBACK_COMPONENT, + { + "output_type": "note", + }, ) serialized["preference_history"] = serialize_preference_history(system_attrs) serialized["preferences"] = get_preferences(system_attrs) diff --git a/optuna_dashboard/ts/action.ts b/optuna_dashboard/ts/action.ts index 0e6d5436..0e1c9420 100644 --- a/optuna_dashboard/ts/action.ts +++ b/optuna_dashboard/ts/action.ts @@ -614,16 +614,22 @@ export const actionCreator = () => { studyId: number, compoennt_type: FeedbackComponentType ) => { - reportFeedbackComponentAPI(studyId, compoennt_type).catch((err) => { - const reason = err.response?.data.reason - enqueueSnackbar( - `Failed to report feedback component. Reason: ${reason}`, - { - variant: "error", - } - ) - console.log(err) - }) + reportFeedbackComponentAPI(studyId, compoennt_type) + .then(() => { + const newStudy = Object.assign({}, studyDetails[studyId]) + newStudy.feedback_component_type = compoennt_type + setStudyDetailState(studyId, newStudy) + }) + .catch((err) => { + const reason = err.response?.data.reason + enqueueSnackbar( + `Failed to report feedback component. Reason: ${reason}`, + { + variant: "error", + } + ) + console.log(err) + }) } return { diff --git a/optuna_dashboard/ts/apiClient.ts b/optuna_dashboard/ts/apiClient.ts index 42c812df..1a0b0f18 100644 --- a/optuna_dashboard/ts/apiClient.ts +++ b/optuna_dashboard/ts/apiClient.ts @@ -1,3 +1,4 @@ +import { Feedback } from "@mui/icons-material" import axios from "axios" const axiosInstance = axios.create({ baseURL: API_ENDPOINT }) @@ -83,30 +84,6 @@ const convertPreferenceHistory = ( } } -interface FeedbackComponentResponse { - type: string - artifact_key?: string -} - -const convertFeedbackComponentType = ( - res?: FeedbackComponentResponse -): FeedbackComponentType => { - if (res === undefined) { - return { - output_type: "note", - } as FeedbackComponentNote - } - if (res.type === "artifact") { - return { - output_type: "artifact", - artifact_key: res.artifact_key, - } as FeedbackComponentArtifact - } - return { - output_type: "note", - } as FeedbackComponentNote -} - interface StudyDetailResponse { name: string datetime_start: string @@ -125,7 +102,7 @@ interface StudyDetailResponse { preferences?: [number, number][] preference_history?: PreferenceHistoryResponce[] plotly_graph_objects: PlotlyGraphObject[] - feedback_component_type?: FeedbackComponentResponse + feedback_component_type?: FeedbackComponentType skipped_trials?: number[] } @@ -162,9 +139,9 @@ export const getStudyDetailAPI = ( objective_names: res.data.objective_names, form_widgets: res.data.form_widgets, is_preferential: res.data.is_preferential, - feedback_component_type: convertFeedbackComponentType( - res.data.feedback_component_type - ), + feedback_component_type: res.data.feedback_component_type ?? { + output_type: "note", + }, preferences: res.data.preferences, preference_history: res.data.preference_history?.map( convertPreferenceHistory diff --git a/optuna_dashboard/ts/components/PreferentialTrials.tsx b/optuna_dashboard/ts/components/PreferentialTrials.tsx index 2777a4a0..4b11edb5 100644 --- a/optuna_dashboard/ts/components/PreferentialTrials.tsx +++ b/optuna_dashboard/ts/components/PreferentialTrials.tsx @@ -75,14 +75,35 @@ const SettingsPage: FC<{ }> = ({ studyDetail, settingShown, setSettingShown }) => { const theme = useTheme() const actions = actionCreator() - const [outputComponent, setOutputComponent] = useState( - studyDetail.feedback_component_type + const [outputComponentType, setOutputComponentType] = useState( + studyDetail.feedback_component_type.output_type + ) + const [artifactKey, setArtifactKey] = useState( + studyDetail.feedback_component_type.output_type === "artifact" + ? studyDetail.feedback_component_type.artifact_key + : undefined ) useEffect(() => { - setOutputComponent(studyDetail.feedback_component_type) - }, [studyDetail.feedback_component_type]) + setOutputComponentType(studyDetail.feedback_component_type.output_type) + }, [studyDetail.feedback_component_type.output_type]) + useEffect(() => { + if (studyDetail.feedback_component_type.output_type === "artifact") { + setArtifactKey(studyDetail.feedback_component_type.artifact_key) + } + }, [ + studyDetail.feedback_component_type.output_type === "artifact" + ? studyDetail.feedback_component_type.artifact_key + : undefined, + ]) const onClose = () => { setSettingShown(false) + const outputComponent: FeedbackComponentType = + outputComponentType === "note" + ? ({ output_type: "note" } as FeedbackComponentNote) + : ({ + output_type: "artifact", + artifact_key: artifactKey, + } as FeedbackComponentArtifact) actions.updateFeedbackComponent(studyDetail.id, outputComponent) } @@ -107,16 +128,16 @@ const SettingsPage: FC<{ Output Component: - {outputComponent.output_type === "artifact" ? ( + {outputComponentType === "artifact" ? ( { - setOutputComponent({ - ...outputComponent, - artifact_key: e.target.value as string, - }) + setArtifactKey(e.target.value) }} > {studyDetail.union_user_attrs.length === 0 ? (