From 1d7d5a7252222b9d6c5d975de36a0826ba3f06f2 Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Mon, 4 Sep 2023 12:15:29 +0900 Subject: [PATCH] fix by review --- optuna_dashboard/_app.py | 11 +++++---- optuna_dashboard/_preferential_history.py | 22 +++++++++++++---- optuna_dashboard/ts/components/AppDrawer.tsx | 2 +- .../ts/components/PreferenceHistory.tsx | 5 ++-- python_tests/test_api.py | 24 +++++++++++++++++++ 5 files changed, 52 insertions(+), 12 deletions(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index f582d5e5..49a60957 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -27,6 +27,7 @@ from ._bottle_util import json_api_view from ._cached_extra_study_property import get_cached_extra_study_property from ._importance import get_param_importance_from_trials_cache from ._pareto_front import get_pareto_front_trials +from ._preferential_history import cast_feedback_mode from ._preferential_history import report_history from ._rdb_migration import register_rdb_migration_route from ._serializer import serialize_study_detail @@ -269,19 +270,19 @@ def create_app( @json_api_view def post_preference(study_id: int) -> dict[str, Any]: try: - mode = request.json.get("mode", "") + mode = cast_feedback_mode(request.json.get("mode", "")) candidates = [int(d) for d in request.json.get("candidates", [])] clicked = int(request.json.get("clicked", -1)) - except ValueError: + except Exception: response.status = 400 return {"reason": "Invalid request."} if clicked == -1: response.status = 400 return {"reason": "`clicked` should be specified."} - if mode != "ChooseWorst": - response.status = 400 - return {"reason": "`mode` should be 'ChooseWorst'."} + # if mode != "ChooseWorst": + # response.status = 400 + # return {"reason": "`mode` should be 'ChooseWorst'."} report_history( study_id, diff --git a/optuna_dashboard/_preferential_history.py b/optuna_dashboard/_preferential_history.py index f4660310..185d3de0 100644 --- a/optuna_dashboard/_preferential_history.py +++ b/optuna_dashboard/_preferential_history.py @@ -3,31 +3,47 @@ from __future__ import annotations from dataclasses import dataclass from datetime import datetime import json +import sys from typing import Any from typing import TYPE_CHECKING import uuid from optuna.storages import BaseStorage +from typeguard import typechecked from .preferential._system_attrs import report_preferences _SYSTEM_ATTR_PREFIX_HISTORY = "preference:history" +if TYPE_CHECKING or (3, 8, 0) <= sys.version_info: + from typing import Literal +else: + from typing_extensions import Literal +FeedbackMode = Literal["ChooseWorst"] if TYPE_CHECKING: from typing import TypedDict - from typing import Literal NewHistoryJSON = TypedDict( "NewHistoryJSON", { - "mode": Literal["ChooseWorst"], + "mode": FeedbackMode, "candidates": list[int], "clicked": int, }, ) +@typechecked +def check_feedback_mode(mode: FeedbackMode) -> None: + pass + + +def cast_feedback_mode(mode: str) -> FeedbackMode: + check_feedback_mode(mode) # type: ignore + return mode # type: ignore + + @dataclass(frozen=True) class ChooseWorstHistory: mode: Literal["ChooseWorst"] @@ -110,8 +126,6 @@ def serialize_preference_history( clicked=choice["clicked"], ) ) - else: - assert False, f"Unknown mode: {choice['mode']}" histories.sort(key=lambda c: c.timestamp) return [history.to_dict() for history in histories] diff --git a/optuna_dashboard/ts/components/AppDrawer.tsx b/optuna_dashboard/ts/components/AppDrawer.tsx index 184ec606..446b362e 100644 --- a/optuna_dashboard/ts/components/AppDrawer.tsx +++ b/optuna_dashboard/ts/components/AppDrawer.tsx @@ -221,7 +221,7 @@ export const AppDrawer: FC<{ component={Link} to={`${URL_PREFIX}/studies/${studyId}/preference-history`} sx={styleListItemButton} - selected={page === "analytics"} + selected={page === "preferenceHistory"} > diff --git a/optuna_dashboard/ts/components/PreferenceHistory.tsx b/optuna_dashboard/ts/components/PreferenceHistory.tsx index e95a4cb1..53cedfbb 100644 --- a/optuna_dashboard/ts/components/PreferenceHistory.tsx +++ b/optuna_dashboard/ts/components/PreferenceHistory.tsx @@ -11,10 +11,11 @@ import ClearIcon from "@mui/icons-material/Clear" import IconButton from "@mui/material/IconButton" import OpenInFullIcon from "@mui/icons-material/OpenInFull" import Modal from "@mui/material/Modal" +import { red } from "@mui/material/colors" import { TrialListDetail } from "./TrialList" import { MarkdownRenderer } from "./Note" -import { red } from "@mui/material/colors" +import { formatDate } from "../dateUtil" type TrialType = "worst" | "none" @@ -149,7 +150,7 @@ const ChoiceTrials: FC<{ choice: PreferenceChoice; trials: Trial[] }> = ({ fontWeight: theme.typography.fontWeightLight, }} > - {choice.timestamp.toISOString()} + {formatDate(choice.timestamp)} None: + storage = optuna.storages.InMemoryStorage() + study = create_study(storage=storage) + for _ in range(3): + trial = study.ask() + study.mark_comparison_ready(trial) + + app = create_app(storage) + study_id = study._study._study_id + status, _, _ = send_request( + app, + f"/api/studies/{study_id}/preference", + "POST", + body=json.dumps( + { + "mode": "ChoseWorst", + "candidates": [0, 1, 2], + "clicked": 1, + } + ), + content_type="application/json", + ) + self.assertEqual(status, 400) + def test_skip_trial(self) -> None: storage = optuna.storages.InMemoryStorage() study = create_study(storage=storage)