From aa35bfa709cd5561894babc1b21b504864f90b1a Mon Sep 17 00:00:00 2001 From: c-bata Date: Thu, 17 Mar 2022 18:07:46 +0900 Subject: [PATCH 01/12] Implement JSON API for note --- optuna_dashboard/_app.py | 20 ++++++ optuna_dashboard/_note.py | 72 +++++++++++++++++++++ optuna_dashboard/_serializer.py | 3 + optuna_dashboard/static/components/Note.tsx | 18 ++++++ python_tests/test_note.py | 19 ++++++ 5 files changed, 132 insertions(+) create mode 100644 optuna_dashboard/_note.py create mode 100644 optuna_dashboard/static/components/Note.tsx create mode 100644 python_tests/test_note.py diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index 831e6093..3caec909 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -38,6 +38,7 @@ from ._intermediate_values import has_intermediate_values from ._search_space import get_search_space from ._serializer import serialize_study_detail from ._serializer import serialize_study_summary +from . import _note as note if typing.TYPE_CHECKING: @@ -281,6 +282,25 @@ def create_app(storage: BaseStorage) -> Bottle: ], } + @app.post("/api/studies//note") + @handle_json_api_exception + def save_note(study_id: int) -> BottleViewReturn: + response.content_type = "application/json" + + system_attrs = storage.get_study_system_attrs(study_id) + req_note_ver = request.json.get("version", None) + req_note_body = request.json.get("body", None) + if req_note_ver is None or req_note_body is None: + response.status = 400 # Bad request + return {"reason": "Invalid request."} + if not note.version_is_incremented(system_attrs, req_note_ver): + response.status = 400 # Bad request + return {"reason": "The text you are editing has changed. Please copy your edits and refresh the page."} + + note.save_note(storage, study_id, req_note_ver, req_note_body) + response.status = 204 # No content + return {} + @app.get("/static/") def send_static(filename: str) -> BottleViewReturn: return static_file(filename, root=STATIC_DIR) diff --git a/optuna_dashboard/_note.py b/optuna_dashboard/_note.py new file mode 100644 index 00000000..8d7fa55b --- /dev/null +++ b/optuna_dashboard/_note.py @@ -0,0 +1,72 @@ +from typing import Dict, Any +from typing import TypedDict +import math + +from optuna.storages import BaseStorage + +SYSTEM_ATTR_MAX_LENGTH = 2045 +NOTE_VER_KEY = "dashboard:note_ver" +NOTE_STR_KEY_PREFIX = "dashboard:note_str:" + +NoteType = TypedDict( + "NoteType", + { + "version": int, + "body": str, + }, +) + + +def get_note_from_system_attrs(system_attrs: Dict[str, Any]) -> NoteType: + if NOTE_VER_KEY not in system_attrs: + return { + "version": 0, + "body": "", + } + note_ver = int(system_attrs[NOTE_VER_KEY]) + note_attrs: Dict[str, str] = { + key: value + for key, value in system_attrs.items() + if key.startswith(NOTE_STR_KEY_PREFIX) + } + return { + "version": note_ver, + "body": concat_body(note_attrs) + } + + +def version_is_incremented(system_attrs: Dict[str, Any], req_note_ver: int) -> bool: + db_note_ver = system_attrs.get(NOTE_VER_KEY, 0) + return req_note_ver == db_note_ver + 1 + + +def save_note(storage: BaseStorage, study_id: int, ver: int, body: str) -> None: + storage.set_study_system_attr(study_id, NOTE_VER_KEY, ver) + + attrs = split_body(body) + for k, v in attrs.items(): + storage.set_study_system_attr(study_id, k, v) + + # Clear previous messages + all_note_attrs: Dict[str, str] = { + key: value + for key, value in storage.get_study_system_attrs(study_id).items() + if key.startswith(NOTE_STR_KEY_PREFIX) + } + if len(all_note_attrs) > len(attrs): + for i in range(len(attrs), len(all_note_attrs)): + storage.set_study_system_attr(study_id, f"{NOTE_STR_KEY_PREFIX}{i}", "") + + +def split_body(note_str: str) -> Dict[str, str]: + note_len = len(note_str) + attrs = {} + for i in range(math.ceil(note_len / SYSTEM_ATTR_MAX_LENGTH)): + start = i * SYSTEM_ATTR_MAX_LENGTH + end = min((i + 1) * SYSTEM_ATTR_MAX_LENGTH, note_len) + attrs[f"{NOTE_STR_KEY_PREFIX}{i}"] = note_str[start:end] + return attrs + + +def concat_body(note_attrs: Dict[str, str]) -> str: + return "".join(note_attrs[f"{NOTE_STR_KEY_PREFIX}{i}"] for i in range(len(note_attrs))) diff --git a/optuna_dashboard/_serializer.py b/optuna_dashboard/_serializer.py index fc155214..d33ec75f 100644 --- a/optuna_dashboard/_serializer.py +++ b/optuna_dashboard/_serializer.py @@ -10,6 +10,8 @@ from optuna.distributions import BaseDistribution from optuna.study import StudySummary from optuna.trial import FrozenTrial +from . import _note as note + try: from typing import TypedDict @@ -111,6 +113,7 @@ def serialize_study_detail( serialized["intersection_search_space"] = serialize_search_space(intersection) serialized["union_search_space"] = serialize_search_space(union) serialized["has_intermediate_values"] = has_intermediate_values + serialized["note"] = note.get_note_from_system_attrs(summary.system_attrs) return serialized diff --git a/optuna_dashboard/static/components/Note.tsx b/optuna_dashboard/static/components/Note.tsx new file mode 100644 index 00000000..2b2a7f3a --- /dev/null +++ b/optuna_dashboard/static/components/Note.tsx @@ -0,0 +1,18 @@ +import {Card, CardContent, useTheme, TextField} from "@mui/material"; +import React, {FC} from "react"; + + +export const Note: FC<{studyId: number}> = ({studyId}) => { + const theme = useTheme() + return ( + + + + + + ) +} \ No newline at end of file diff --git a/python_tests/test_note.py b/python_tests/test_note.py new file mode 100644 index 00000000..f69de768 --- /dev/null +++ b/python_tests/test_note.py @@ -0,0 +1,19 @@ +from unittest import TestCase +from unittest.mock import patch + +from optuna_dashboard import _note as note + + +class NoteTestCase(TestCase): + @patch("optuna_dashboard._note.SYSTEM_ATTR_MAX_LENGTH", 5) + def test_split_and_concat_note_body(self) -> None: + for dummy_body_str, attr_len in [ + ("012", 1), + ("01234", 1), + ("012345", 2), + ]: + with self.subTest(f"with_{dummy_body_str}_{attr_len}"): + attrs = note.split_body(dummy_body_str) + assert len(attrs) == attr_len + actual = note.concat_body(attrs) + assert dummy_body_str == actual From 0c18b2335fd5844165ccf1c6401c237635cf7fd4 Mon Sep 17 00:00:00 2001 From: c-bata Date: Fri, 18 Mar 2022 13:34:48 +0900 Subject: [PATCH 02/12] Implement API client for note --- optuna_dashboard/static/apiClient.ts | 17 +++++++++++++++++ optuna_dashboard/static/types/index.d.ts | 7 +++++++ 2 files changed, 24 insertions(+) diff --git a/optuna_dashboard/static/apiClient.ts b/optuna_dashboard/static/apiClient.ts index 6dd934a4..ab76c74e 100644 --- a/optuna_dashboard/static/apiClient.ts +++ b/optuna_dashboard/static/apiClient.ts @@ -45,6 +45,10 @@ interface StudyDetailResponse { intersection_search_space: SearchSpace[] union_search_space: SearchSpace[] has_intermediate_values: boolean + note: { + version: number + body: string + } } export const getStudyDetailAPI = ( @@ -62,6 +66,7 @@ export const getStudyDetailAPI = ( return convertTrialResponse(trial) }) return { + id: studyId, name: res.data.name, datetime_start: new Date(res.data.datetime_start), directions: res.data.directions, @@ -72,6 +77,7 @@ export const getStudyDetailAPI = ( union_search_space: res.data.union_search_space, intersection_search_space: res.data.intersection_search_space, has_intermediate_values: res.data.has_intermediate_values, + note: res.data.note, } }) } @@ -178,6 +184,17 @@ export const deleteStudyAPI = (studyId: number) => { }) } +export const saveNoteAPI = ( + studyId: number, + note: { version: number; body: string } +): Promise => { + return axiosInstance + .post(`/api/studies/${studyId}/note`, note) + .then((res) => { + return + }) +} + interface ParamImportancesResponse { target_name: string param_importances: ParamImportance[] diff --git a/optuna_dashboard/static/types/index.d.ts b/optuna_dashboard/static/types/index.d.ts index 4e62e683..d62102ce 100644 --- a/optuna_dashboard/static/types/index.d.ts +++ b/optuna_dashboard/static/types/index.d.ts @@ -45,6 +45,11 @@ declare interface Attribute { value: string } +declare interface Note { + version: number + body: string +} + declare interface Trial { trial_id: number study_id: number @@ -70,6 +75,7 @@ declare interface StudySummary { } declare interface StudyDetail { + id: number name: string directions: StudyDirection[] datetime_start: Date @@ -78,6 +84,7 @@ declare interface StudyDetail { intersection_search_space: SearchSpace[] union_search_space: SearchSpace[] has_intermediate_values: boolean + note: Note } declare interface StudyDetails { From 40c2aab767bb83f770ea4d5121ee247a5b4f3bfc Mon Sep 17 00:00:00 2001 From: c-bata Date: Fri, 18 Mar 2022 14:47:34 +0900 Subject: [PATCH 03/12] Implement Note editor --- optuna_dashboard/static/action.ts | 22 ++ optuna_dashboard/static/components/Note.tsx | 103 ++++++- .../static/components/StudyDetail.tsx | 9 + package-lock.json | 286 ++++++++++++++++-- package.json | 1 + 5 files changed, 384 insertions(+), 37 deletions(-) diff --git a/optuna_dashboard/static/action.ts b/optuna_dashboard/static/action.ts index 5f1061c9..4f398497 100644 --- a/optuna_dashboard/static/action.ts +++ b/optuna_dashboard/static/action.ts @@ -5,6 +5,7 @@ import { getStudySummariesAPI, createNewStudyAPI, deleteStudyAPI, + saveNoteAPI, } from "./apiClient" import { studyDetailsState, studySummariesState } from "./state" @@ -95,11 +96,32 @@ export const actionCreator = () => { }) } + const saveNote = (studyId: number, note: Note) => { + saveNoteAPI(studyId, note) + .then(() => { + const newStudy = Object.assign({}, studyDetails[studyId]) + newStudy.note = note + const newStudies = Object.assign({}, studyDetails) + newStudies[studyId] = newStudy + setStudyDetails(newStudies) + enqueueSnackbar(`Success to save the note`, { + variant: "success", + }) + }) + .catch((err) => { + const reason = err.response?.data.reason + enqueueSnackbar(`Failed: ${reason}`, { + variant: "error", + }) + }) + } + return { updateStudyDetail, updateStudySummaries, createNewStudy, deleteStudy, + saveNote, } } diff --git a/optuna_dashboard/static/components/Note.tsx b/optuna_dashboard/static/components/Note.tsx index 2b2a7f3a..0e01d1a9 100644 --- a/optuna_dashboard/static/components/Note.tsx +++ b/optuna_dashboard/static/components/Note.tsx @@ -1,18 +1,91 @@ -import {Card, CardContent, useTheme, TextField} from "@mui/material"; -import React, {FC} from "react"; +import { Box, Button, TextField, Typography, useTheme } from "@mui/material" +import React, { FC, createRef, useState, useEffect } from "react" +import LoadingButton from "@mui/lab/LoadingButton" +import SaveIcon from "@mui/icons-material/Save" +import { actionCreator } from "../action" -export const Note: FC<{studyId: number}> = ({studyId}) => { - const theme = useTheme() - return ( - = ({ studyId, latestNote }) => { + const theme = useTheme() + const [disable, setDisable] = useState(true) + const [curNote, setCurNote] = useState({ version: 0, body: "" }) + const textAreaRef = createRef() + const action = actionCreator() + const notLatest = latestNote.version > curNote.version + + useEffect(() => { + setCurNote(latestNote) + }, []) + const handleSave = () => { + const newNote = { + version: curNote.version + 1, + body: textAreaRef.current ? textAreaRef.current.value : "" + } + setCurNote(newNote) + action.saveNote(studyId, newNote) + } + const handleRefresh = () => { + if (!textAreaRef.current) { + console.log("Unexpectedly, textarea is not found.") + return + } + textAreaRef.current.value = latestNote.body + setCurNote(latestNote) + } + + return ( + <> + { + const cur = textAreaRef.current ? textAreaRef.current.value : "" + setDisable(cur === latestNote.body) + }} + /> + + {notLatest && ( + <> + + The text you are editing has updated. Do you want to discard your + changes and refresh the textarea? + + + + )} + + } + variant="contained" + disabled={disable} > - - - - - ) -} \ No newline at end of file + Save + + + + ) +} diff --git a/optuna_dashboard/static/components/StudyDetail.tsx b/optuna_dashboard/static/components/StudyDetail.tsx index b7f4abc4..1796bcec 100644 --- a/optuna_dashboard/static/components/StudyDetail.tsx +++ b/optuna_dashboard/static/components/StudyDetail.tsx @@ -37,6 +37,7 @@ import { GraphIntermediateValues } from "./GraphIntermediateValues" import { GraphSlice } from "./GraphSlice" import { GraphHistory } from "./GraphHistory" import { GraphParetoFront } from "./GraphParetoFront" +import { Note } from "./Note" import { actionCreator } from "../action" import { studyDetailsState } from "../state" @@ -382,6 +383,14 @@ export const StudyDetail: FC<{ + + + Note + {studyDetail !== null && ( + + )} + + diff --git a/package-lock.json b/package-lock.json index 90d2df10..4c468579 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,6 +10,7 @@ "license": "MIT", "dependencies": { "@mui/icons-material": "^5.4.4", + "@mui/lab": "^5.0.0-alpha.73", "@mui/material": "^5.4.4", "axios": "^0.21.2", "notistack": "^2.0.3", @@ -2740,6 +2741,75 @@ "integrity": "sha512-0hYQ8SB4Db5zvZB4axdMHGwEaQjkZzFjQiN9LVYvIFB2nSUHW9tYpxWriPrWDASIxiaXax83REcLxuSdnGPZtw==", "dev": true }, + "node_modules/@date-io/core": { + "version": "2.13.1", + "resolved": "https://registry.npmjs.org/@date-io/core/-/core-2.13.1.tgz", + "integrity": "sha512-pVI9nfkf2qClb2Cxdq0Q4zJhdawMG4ybWZUVGifT78FDwzRMX2SwXBb55s5NRJk0HcIicDuxktmCtemZqMH1Zg==" + }, + "node_modules/@date-io/date-fns": { + "version": "2.13.1", + "resolved": "https://registry.npmjs.org/@date-io/date-fns/-/date-fns-2.13.1.tgz", + "integrity": "sha512-8fmfwjiLMpFLD+t4NBwDx0eblWnNcgt4NgfT/uiiQTGI81fnPu9tpBMYdAcuWxaV7LLpXgzLBx1SYWAMDVUDQQ==", + "dependencies": { + "@date-io/core": "^2.13.1" + }, + "peerDependencies": { + "date-fns": "^2.0.0" + }, + "peerDependenciesMeta": { + "date-fns": { + "optional": true + } + } + }, + "node_modules/@date-io/dayjs": { + "version": "2.13.1", + "resolved": "https://registry.npmjs.org/@date-io/dayjs/-/dayjs-2.13.1.tgz", + "integrity": "sha512-5bL4WWWmlI4uGZVScANhHJV7Mjp93ec2gNeUHDqqLaMZhp51S0NgD25oqj/k0LqBn1cdU2MvzNpk/ObMmVv5cQ==", + "dependencies": { + "@date-io/core": "^2.13.1" + }, + "peerDependencies": { + "dayjs": "^1.8.17" + }, + "peerDependenciesMeta": { + "dayjs": { + "optional": true + } + } + }, + "node_modules/@date-io/luxon": { + "version": "2.13.1", + "resolved": "https://registry.npmjs.org/@date-io/luxon/-/luxon-2.13.1.tgz", + "integrity": "sha512-yG+uM7lXfwLyKKEwjvP8oZ7qblpmfl9gxQYae55ifbwiTs0CoCTkYkxEaQHGkYtTqGTzLqcb0O9Pzx6vgWg+yg==", + "dependencies": { + "@date-io/core": "^2.13.1" + }, + "peerDependencies": { + "luxon": "^1.21.3 || ^2.x" + }, + "peerDependenciesMeta": { + "luxon": { + "optional": true + } + } + }, + "node_modules/@date-io/moment": { + "version": "2.13.1", + "resolved": "https://registry.npmjs.org/@date-io/moment/-/moment-2.13.1.tgz", + "integrity": "sha512-XX1X/Tlvl3TdqQy2j0ZUtEJV6Rl8tOyc5WOS3ki52He28Uzme4Ro/JuPWTMBDH63weSWIZDlbR7zBgp3ZA2y1A==", + "dependencies": { + "@date-io/core": "^2.13.1" + }, + "peerDependencies": { + "moment": "^2.24.0" + }, + "peerDependenciesMeta": { + "moment": { + "optional": true + } + } + }, "node_modules/@discoveryjs/json-ext": { "version": "0.5.3", "resolved": "https://registry.npmjs.org/@discoveryjs/json-ext/-/json-ext-0.5.3.tgz", @@ -3681,6 +3751,91 @@ } } }, + "node_modules/@mui/lab": { + "version": "5.0.0-alpha.73", + "resolved": "https://registry.npmjs.org/@mui/lab/-/lab-5.0.0-alpha.73.tgz", + "integrity": "sha512-10Uj0Atc7gBTXKX4VV38P6RdqTQrJZxcl3HeEcytIO1S3NAGfc7gZ3Hdpnhtj5U8kcRJZZPH9LtrBbMZzxU/1A==", + "dependencies": { + "@babel/runtime": "^7.17.2", + "@date-io/date-fns": "^2.13.1", + "@date-io/dayjs": "^2.13.1", + "@date-io/luxon": "^2.13.1", + "@date-io/moment": "^2.13.1", + "@mui/base": "5.0.0-alpha.72", + "@mui/system": "^5.5.1", + "@mui/utils": "^5.4.4", + "clsx": "^1.1.1", + "prop-types": "^15.7.2", + "react-is": "^17.0.2", + "react-transition-group": "^4.4.2", + "rifm": "^0.12.1" + }, + "engines": { + "node": ">=12.0.0" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/mui" + }, + "peerDependencies": { + "@mui/material": "^5.0.0", + "@types/react": "^16.8.6 || ^17.0.0", + "date-fns": "^2.25.0", + "dayjs": "^1.10.7", + "luxon": "^1.28.0 || ^2.0.0", + "moment": "^2.29.1", + "react": "^17.0.0", + "react-dom": "^17.0.0" + }, + "peerDependenciesMeta": { + "@types/react": { + "optional": true + }, + "date-fns": { + "optional": true + }, + "dayjs": { + "optional": true + }, + "luxon": { + "optional": true + }, + "moment": { + "optional": true + } + } + }, + "node_modules/@mui/lab/node_modules/@mui/base": { + "version": "5.0.0-alpha.72", + "resolved": "https://registry.npmjs.org/@mui/base/-/base-5.0.0-alpha.72.tgz", + "integrity": "sha512-WCAooa9eqbsC68LhyKtDBRumH4hV1eRZ0A3SDKFHSwYG9fCOdsFv/H1dIYRJM0rwD45bMnuDiG3Qmx7YsTiptw==", + "dependencies": { + "@babel/runtime": "^7.17.2", + "@emotion/is-prop-valid": "^1.1.2", + "@mui/utils": "^5.4.4", + "@popperjs/core": "^2.11.3", + "clsx": "^1.1.1", + "prop-types": "^15.7.2", + "react-is": "^17.0.2" + }, + "engines": { + "node": ">=12.0.0" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/mui" + }, + "peerDependencies": { + "@types/react": "^16.8.6 || ^17.0.0", + "react": "^17.0.0", + "react-dom": "^17.0.0" + }, + "peerDependenciesMeta": { + "@types/react": { + "optional": true + } + } + }, "node_modules/@mui/material": { "version": "5.4.4", "resolved": "https://registry.npmjs.org/@mui/material/-/material-5.4.4.tgz", @@ -3782,17 +3937,17 @@ } }, "node_modules/@mui/system": { - "version": "5.4.4", - "resolved": "https://registry.npmjs.org/@mui/system/-/system-5.4.4.tgz", - "integrity": "sha512-Zjbztq2o/VRuRRCWjG44juRrPKYLQMqtQpMHmMttGu5BnvK6PAPW3WOY0r1JCAwLhbd8Kug9nyhGQYKETjo+tQ==", + "version": "5.5.1", + "resolved": "https://registry.npmjs.org/@mui/system/-/system-5.5.1.tgz", + "integrity": "sha512-2hynI4hN8304hOCT8sc4knJviwUUYJ7XK3mXwQ0nagVGOPnWSOad/nYADm7K0vdlCeUXLIbDbe7oNN3Kaiu2kA==", "dependencies": { "@babel/runtime": "^7.17.2", "@mui/private-theming": "^5.4.4", "@mui/styled-engine": "^5.4.4", - "@mui/types": "^7.1.2", + "@mui/types": "^7.1.3", "@mui/utils": "^5.4.4", "clsx": "^1.1.1", - "csstype": "^3.0.10", + "csstype": "^3.0.11", "prop-types": "^15.7.2" }, "engines": { @@ -3821,9 +3976,9 @@ } }, "node_modules/@mui/types": { - "version": "7.1.2", - "resolved": "https://registry.npmjs.org/@mui/types/-/types-7.1.2.tgz", - "integrity": "sha512-SD7O1nVzqG+ckQpFjDhXPZjRceB8HQFHEvdLLrPhlJy4lLbwEBbxK74Tj4t6Jgk0fTvLJisuwOutrtYe9P/xBQ==", + "version": "7.1.3", + "resolved": "https://registry.npmjs.org/@mui/types/-/types-7.1.3.tgz", + "integrity": "sha512-DDF0UhMBo4Uezlk+6QxrlDbchF79XG6Zs0zIewlR4c0Dt6GKVFfUtzPtHCH1tTbcSlq/L2bGEdiaoHBJ9Y1gSA==", "peerDependencies": { "@types/react": "*" }, @@ -3891,9 +4046,9 @@ } }, "node_modules/@popperjs/core": { - "version": "2.11.2", - "resolved": "https://registry.npmjs.org/@popperjs/core/-/core-2.11.2.tgz", - "integrity": "sha512-92FRmppjjqz29VMJ2dn+xdyXZBrMlE42AV6Kq6BwjWV7CNUW1hs2FtxSNLQE+gJhaZ6AAmYuO9y8dshhcBl7vA==", + "version": "2.11.4", + "resolved": "https://registry.npmjs.org/@popperjs/core/-/core-2.11.4.tgz", + "integrity": "sha512-q/ytXxO5NKvyT37pmisQAItCFqA7FD/vNb8dgaJy3/630Fsc+Mz9/9f2SziBoIZ30TJooXyTwZmhi1zjXmObYg==", "funding": { "type": "opencollective", "url": "https://opencollective.com/popperjs" @@ -10287,6 +10442,14 @@ "node": ">=0.10.0" } }, + "node_modules/rifm": { + "version": "0.12.1", + "resolved": "https://registry.npmjs.org/rifm/-/rifm-0.12.1.tgz", + "integrity": "sha512-OGA1Bitg/dSJtI/c4dh90svzaUPt228kzFsUkJbtA2c964IqEAwWXeL9ZJi86xWv3j5SMqRvGULl7bA6cK0Bvg==", + "peerDependencies": { + "react": ">=16.8" + } + }, "node_modules/rimraf": { "version": "3.0.2", "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-3.0.2.tgz", @@ -13674,6 +13837,43 @@ "integrity": "sha512-0hYQ8SB4Db5zvZB4axdMHGwEaQjkZzFjQiN9LVYvIFB2nSUHW9tYpxWriPrWDASIxiaXax83REcLxuSdnGPZtw==", "dev": true }, + "@date-io/core": { + "version": "2.13.1", + "resolved": "https://registry.npmjs.org/@date-io/core/-/core-2.13.1.tgz", + "integrity": "sha512-pVI9nfkf2qClb2Cxdq0Q4zJhdawMG4ybWZUVGifT78FDwzRMX2SwXBb55s5NRJk0HcIicDuxktmCtemZqMH1Zg==" + }, + "@date-io/date-fns": { + "version": "2.13.1", + "resolved": "https://registry.npmjs.org/@date-io/date-fns/-/date-fns-2.13.1.tgz", + "integrity": "sha512-8fmfwjiLMpFLD+t4NBwDx0eblWnNcgt4NgfT/uiiQTGI81fnPu9tpBMYdAcuWxaV7LLpXgzLBx1SYWAMDVUDQQ==", + "requires": { + "@date-io/core": "^2.13.1" + } + }, + "@date-io/dayjs": { + "version": "2.13.1", + "resolved": "https://registry.npmjs.org/@date-io/dayjs/-/dayjs-2.13.1.tgz", + "integrity": "sha512-5bL4WWWmlI4uGZVScANhHJV7Mjp93ec2gNeUHDqqLaMZhp51S0NgD25oqj/k0LqBn1cdU2MvzNpk/ObMmVv5cQ==", + "requires": { + "@date-io/core": "^2.13.1" + } + }, + "@date-io/luxon": { + "version": "2.13.1", + "resolved": "https://registry.npmjs.org/@date-io/luxon/-/luxon-2.13.1.tgz", + "integrity": "sha512-yG+uM7lXfwLyKKEwjvP8oZ7qblpmfl9gxQYae55ifbwiTs0CoCTkYkxEaQHGkYtTqGTzLqcb0O9Pzx6vgWg+yg==", + "requires": { + "@date-io/core": "^2.13.1" + } + }, + "@date-io/moment": { + "version": "2.13.1", + "resolved": "https://registry.npmjs.org/@date-io/moment/-/moment-2.13.1.tgz", + "integrity": "sha512-XX1X/Tlvl3TdqQy2j0ZUtEJV6Rl8tOyc5WOS3ki52He28Uzme4Ro/JuPWTMBDH63weSWIZDlbR7zBgp3ZA2y1A==", + "requires": { + "@date-io/core": "^2.13.1" + } + }, "@discoveryjs/json-ext": { "version": "0.5.3", "resolved": "https://registry.npmjs.org/@discoveryjs/json-ext/-/json-ext-0.5.3.tgz", @@ -14375,6 +14575,42 @@ "@babel/runtime": "^7.17.2" } }, + "@mui/lab": { + "version": "5.0.0-alpha.73", + "resolved": "https://registry.npmjs.org/@mui/lab/-/lab-5.0.0-alpha.73.tgz", + "integrity": "sha512-10Uj0Atc7gBTXKX4VV38P6RdqTQrJZxcl3HeEcytIO1S3NAGfc7gZ3Hdpnhtj5U8kcRJZZPH9LtrBbMZzxU/1A==", + "requires": { + "@babel/runtime": "^7.17.2", + "@date-io/date-fns": "^2.13.1", + "@date-io/dayjs": "^2.13.1", + "@date-io/luxon": "^2.13.1", + "@date-io/moment": "^2.13.1", + "@mui/base": "5.0.0-alpha.72", + "@mui/system": "^5.5.1", + "@mui/utils": "^5.4.4", + "clsx": "^1.1.1", + "prop-types": "^15.7.2", + "react-is": "^17.0.2", + "react-transition-group": "^4.4.2", + "rifm": "^0.12.1" + }, + "dependencies": { + "@mui/base": { + "version": "5.0.0-alpha.72", + "resolved": "https://registry.npmjs.org/@mui/base/-/base-5.0.0-alpha.72.tgz", + "integrity": "sha512-WCAooa9eqbsC68LhyKtDBRumH4hV1eRZ0A3SDKFHSwYG9fCOdsFv/H1dIYRJM0rwD45bMnuDiG3Qmx7YsTiptw==", + "requires": { + "@babel/runtime": "^7.17.2", + "@emotion/is-prop-valid": "^1.1.2", + "@mui/utils": "^5.4.4", + "@popperjs/core": "^2.11.3", + "clsx": "^1.1.1", + "prop-types": "^15.7.2", + "react-is": "^17.0.2" + } + } + } + }, "@mui/material": { "version": "5.4.4", "resolved": "https://registry.npmjs.org/@mui/material/-/material-5.4.4.tgz", @@ -14415,24 +14651,24 @@ } }, "@mui/system": { - "version": "5.4.4", - "resolved": "https://registry.npmjs.org/@mui/system/-/system-5.4.4.tgz", - "integrity": "sha512-Zjbztq2o/VRuRRCWjG44juRrPKYLQMqtQpMHmMttGu5BnvK6PAPW3WOY0r1JCAwLhbd8Kug9nyhGQYKETjo+tQ==", + "version": "5.5.1", + "resolved": "https://registry.npmjs.org/@mui/system/-/system-5.5.1.tgz", + "integrity": "sha512-2hynI4hN8304hOCT8sc4knJviwUUYJ7XK3mXwQ0nagVGOPnWSOad/nYADm7K0vdlCeUXLIbDbe7oNN3Kaiu2kA==", "requires": { "@babel/runtime": "^7.17.2", "@mui/private-theming": "^5.4.4", "@mui/styled-engine": "^5.4.4", - "@mui/types": "^7.1.2", + "@mui/types": "^7.1.3", "@mui/utils": "^5.4.4", "clsx": "^1.1.1", - "csstype": "^3.0.10", + "csstype": "^3.0.11", "prop-types": "^15.7.2" } }, "@mui/types": { - "version": "7.1.2", - "resolved": "https://registry.npmjs.org/@mui/types/-/types-7.1.2.tgz", - "integrity": "sha512-SD7O1nVzqG+ckQpFjDhXPZjRceB8HQFHEvdLLrPhlJy4lLbwEBbxK74Tj4t6Jgk0fTvLJisuwOutrtYe9P/xBQ==", + "version": "7.1.3", + "resolved": "https://registry.npmjs.org/@mui/types/-/types-7.1.3.tgz", + "integrity": "sha512-DDF0UhMBo4Uezlk+6QxrlDbchF79XG6Zs0zIewlR4c0Dt6GKVFfUtzPtHCH1tTbcSlq/L2bGEdiaoHBJ9Y1gSA==", "requires": {} }, "@mui/utils": { @@ -14474,9 +14710,9 @@ } }, "@popperjs/core": { - "version": "2.11.2", - "resolved": "https://registry.npmjs.org/@popperjs/core/-/core-2.11.2.tgz", - "integrity": "sha512-92FRmppjjqz29VMJ2dn+xdyXZBrMlE42AV6Kq6BwjWV7CNUW1hs2FtxSNLQE+gJhaZ6AAmYuO9y8dshhcBl7vA==" + "version": "2.11.4", + "resolved": "https://registry.npmjs.org/@popperjs/core/-/core-2.11.4.tgz", + "integrity": "sha512-q/ytXxO5NKvyT37pmisQAItCFqA7FD/vNb8dgaJy3/630Fsc+Mz9/9f2SziBoIZ30TJooXyTwZmhi1zjXmObYg==" }, "@sinonjs/commons": { "version": "1.8.3", @@ -19265,6 +19501,12 @@ "integrity": "sha512-U9nH88a3fc/ekCF1l0/UP1IosiuIjyTh7hBvXVMHYgVcfGvt897Xguj2UOLDeI5BG2m7/uwyaLVT6fbtCwTyzw==", "dev": true }, + "rifm": { + "version": "0.12.1", + "resolved": "https://registry.npmjs.org/rifm/-/rifm-0.12.1.tgz", + "integrity": "sha512-OGA1Bitg/dSJtI/c4dh90svzaUPt228kzFsUkJbtA2c964IqEAwWXeL9ZJi86xWv3j5SMqRvGULl7bA6cK0Bvg==", + "requires": {} + }, "rimraf": { "version": "3.0.2", "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-3.0.2.tgz", diff --git a/package.json b/package.json index 3338baf6..39663336 100644 --- a/package.json +++ b/package.json @@ -17,6 +17,7 @@ "license": "MIT", "dependencies": { "@mui/icons-material": "^5.4.4", + "@mui/lab": "^5.0.0-alpha.73", "@mui/material": "^5.4.4", "axios": "^0.21.2", "notistack": "^2.0.3", From b366191d828aad0c3e21da23ffb4e3a6fb24f73b Mon Sep 17 00:00:00 2001 From: c-bata Date: Sat, 19 Mar 2022 02:27:20 +0900 Subject: [PATCH 04/12] Add saving state --- optuna_dashboard/_app.py | 11 +++++---- optuna_dashboard/static/action.ts | 5 ++-- optuna_dashboard/static/apiClient.ts | 2 +- optuna_dashboard/static/components/Note.tsx | 24 +++++++++++++------ .../static/components/StudyDetail.tsx | 7 +++++- 5 files changed, 34 insertions(+), 15 deletions(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index 3caec909..50762650 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -282,20 +282,23 @@ def create_app(storage: BaseStorage) -> Bottle: ], } - @app.post("/api/studies//note") + @app.put("/api/studies//note") @handle_json_api_exception def save_note(study_id: int) -> BottleViewReturn: response.content_type = "application/json" - system_attrs = storage.get_study_system_attrs(study_id) req_note_ver = request.json.get("version", None) req_note_body = request.json.get("body", None) if req_note_ver is None or req_note_body is None: response.status = 400 # Bad request return {"reason": "Invalid request."} + + system_attrs = storage.get_study_system_attrs(study_id) if not note.version_is_incremented(system_attrs, req_note_ver): - response.status = 400 # Bad request - return {"reason": "The text you are editing has changed. Please copy your edits and refresh the page."} + response.status = 409 # Conflict + return { + "reason": "The text you are editing has changed. Please copy your edits and refresh the page.", + } note.save_note(storage, study_id, req_note_ver, req_note_body) response.status = 204 # No content diff --git a/optuna_dashboard/static/action.ts b/optuna_dashboard/static/action.ts index 4f398497..76118c3f 100644 --- a/optuna_dashboard/static/action.ts +++ b/optuna_dashboard/static/action.ts @@ -96,8 +96,8 @@ export const actionCreator = () => { }) } - const saveNote = (studyId: number, note: Note) => { - saveNoteAPI(studyId, note) + const saveNote = (studyId: number, note: Note): Promise => { + return saveNoteAPI(studyId, note) .then(() => { const newStudy = Object.assign({}, studyDetails[studyId]) newStudy.note = note @@ -113,6 +113,7 @@ export const actionCreator = () => { enqueueSnackbar(`Failed: ${reason}`, { variant: "error", }) + throw err }) } diff --git a/optuna_dashboard/static/apiClient.ts b/optuna_dashboard/static/apiClient.ts index ab76c74e..55495b77 100644 --- a/optuna_dashboard/static/apiClient.ts +++ b/optuna_dashboard/static/apiClient.ts @@ -189,7 +189,7 @@ export const saveNoteAPI = ( note: { version: number; body: string } ): Promise => { return axiosInstance - .post(`/api/studies/${studyId}/note`, note) + .put(`/api/studies/${studyId}/note`, note) .then((res) => { return }) diff --git a/optuna_dashboard/static/components/Note.tsx b/optuna_dashboard/static/components/Note.tsx index 0e01d1a9..24b7bf7c 100644 --- a/optuna_dashboard/static/components/Note.tsx +++ b/optuna_dashboard/static/components/Note.tsx @@ -10,6 +10,7 @@ export const Note: FC<{ latestNote: Note }> = ({ studyId, latestNote }) => { const theme = useTheme() + const [saving, setSaving] = useState(false) const [disable, setDisable] = useState(true) const [curNote, setCurNote] = useState({ version: 0, body: "" }) const textAreaRef = createRef() @@ -20,12 +21,20 @@ export const Note: FC<{ setCurNote(latestNote) }, []) const handleSave = () => { + const nextVersion = curNote.version + 1 const newNote = { - version: curNote.version + 1, - body: textAreaRef.current ? textAreaRef.current.value : "" + version: nextVersion, + body: textAreaRef.current ? textAreaRef.current.value : "", } - setCurNote(newNote) - action.saveNote(studyId, newNote) + setSaving(true) + action + .saveNote(studyId, newNote) + .then(() => { + setCurNote(newNote) + }) + .finally(() => { + setSaving(false) + }) } const handleRefresh = () => { if (!textAreaRef.current) { @@ -39,6 +48,7 @@ export const Note: FC<{ return ( <> { const cur = textAreaRef.current ? textAreaRef.current.value : "" - setDisable(cur === latestNote.body) + setDisable(cur === curNote.body) }} /> - {notLatest && ( + {notLatest && !saving && ( <> } variant="contained" diff --git a/optuna_dashboard/static/components/StudyDetail.tsx b/optuna_dashboard/static/components/StudyDetail.tsx index 1796bcec..1373e5ed 100644 --- a/optuna_dashboard/static/components/StudyDetail.tsx +++ b/optuna_dashboard/static/components/StudyDetail.tsx @@ -385,7 +385,12 @@ export const StudyDetail: FC<{ - Note + + Note + {studyDetail !== null && ( )} From e96b560f2298d6f86e633a689a6b2e35e6b3d6a3 Mon Sep 17 00:00:00 2001 From: c-bata Date: Sat, 19 Mar 2022 02:44:41 +0900 Subject: [PATCH 05/12] Add preference checkbox for Note --- optuna_dashboard/static/components/Note.tsx | 117 ++++++++++-------- .../static/components/StudyDetail.tsx | 53 ++++---- 2 files changed, 94 insertions(+), 76 deletions(-) diff --git a/optuna_dashboard/static/components/Note.tsx b/optuna_dashboard/static/components/Note.tsx index 24b7bf7c..0ba538b7 100644 --- a/optuna_dashboard/static/components/Note.tsx +++ b/optuna_dashboard/static/components/Note.tsx @@ -1,4 +1,12 @@ -import { Box, Button, TextField, Typography, useTheme } from "@mui/material" +import { + Box, + Button, + Card, + CardContent, + TextField, + Typography, + useTheme, +} from "@mui/material" import React, { FC, createRef, useState, useEffect } from "react" import LoadingButton from "@mui/lab/LoadingButton" import SaveIcon from "@mui/icons-material/Save" @@ -46,56 +54,63 @@ export const Note: FC<{ } return ( - <> - { - const cur = textAreaRef.current ? textAreaRef.current.value : "" - setDisable(cur === curNote.body) - }} - /> - - {notLatest && !saving && ( - <> - - The text you are editing has updated. Do you want to discard your - changes and refresh the textarea? - - - - )} - - } - variant="contained" - disabled={disable} + + + + Note + + { + const cur = textAreaRef.current ? textAreaRef.current.value : "" + setDisable(cur === curNote.body) + }} + /> + - Save - - - + {notLatest && !saving && ( + <> + + The text you are editing has updated. Do you want to discard + your changes and refresh the textarea? + + + + )} + + } + variant="contained" + disabled={disable} + > + Save + + + + ) } diff --git a/optuna_dashboard/static/components/StudyDetail.tsx b/optuna_dashboard/static/components/StudyDetail.tsx index 1373e5ed..a18e22df 100644 --- a/optuna_dashboard/static/components/StudyDetail.tsx +++ b/optuna_dashboard/static/components/StudyDetail.tsx @@ -59,9 +59,10 @@ interface Preference { graphParetoFrontChecked: boolean graphParallelCoordinateChecked: boolean graphIntermediateValuesChecked: boolean - edfChecked: boolean + graphEdfChecked: boolean graphHyperparameterImportancesChecked: boolean graphSliceChecked: boolean + noteEditorChecked: boolean reloadInterval: number } @@ -79,9 +80,10 @@ export const StudyDetail: FC<{ graphParetoFrontChecked: true, graphParallelCoordinateChecked: true, graphIntermediateValuesChecked: true, - edfChecked: true, + graphEdfChecked: true, graphHyperparameterImportancesChecked: true, graphSliceChecked: true, + noteEditorChecked: true, reloadInterval: 10, }) useEffect(() => { @@ -102,7 +104,7 @@ export const StudyDetail: FC<{ const handleClose = () => { setPrefOpen(false) } - const handleChartShownChange = ( + const handlePreferenceOnChange = ( event: React.ChangeEvent ) => { setPreferences({ @@ -160,7 +162,7 @@ export const StudyDetail: FC<{ control={ } @@ -173,7 +175,7 @@ export const StudyDetail: FC<{ control={ } @@ -183,7 +185,7 @@ export const StudyDetail: FC<{ control={ } @@ -198,7 +200,7 @@ export const StudyDetail: FC<{ control={ } @@ -207,8 +209,8 @@ export const StudyDetail: FC<{ } @@ -218,7 +220,7 @@ export const StudyDetail: FC<{ control={ } @@ -228,12 +230,23 @@ export const StudyDetail: FC<{ control={ } label="Slice" /> + Editor + + } + label="NoteEditor" + /> @@ -355,7 +368,7 @@ export const StudyDetail: FC<{ ) : null} - {preferences.edfChecked ? ( + {preferences.graphEdfChecked ? ( @@ -383,19 +396,9 @@ export const StudyDetail: FC<{ - - - - Note - - {studyDetail !== null && ( - - )} - - + {studyDetail !== null && preferences.noteEditorChecked ? ( + + ) : null} From 836e0c741ae9bb505007dc4ecb753eeb57b0f895 Mon Sep 17 00:00:00 2001 From: c-bata Date: Sat, 19 Mar 2022 02:45:25 +0900 Subject: [PATCH 06/12] Refactor bottle code --- optuna_dashboard/_app.py | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index 50762650..66060234 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -72,10 +72,11 @@ trials_cache: Dict[int, List[FrozenTrial]] = {} trials_last_fetched_at: Dict[int, datetime] = {} -def handle_json_api_exception(view: BottleView) -> BottleView: +def json_api_view(view: BottleView) -> BottleView: @functools.wraps(view) def decorated(*args: List[Any], **kwargs: Dict[str, Any]) -> BottleViewReturn: try: + response.content_type = "application/json" response_body = view(*args, **kwargs) return response_body except Exception as e: @@ -141,9 +142,8 @@ def create_app(storage: BaseStorage) -> Bottle: return INDEX_HTML @app.get("/api/studies") - @handle_json_api_exception + @json_api_view def list_study_summaries() -> BottleViewReturn: - response.content_type = "application/json" summaries = [ serialize_study_summary(summary) for summary in storage.get_all_study_summaries() @@ -153,10 +153,8 @@ def create_app(storage: BaseStorage) -> Bottle: } @app.post("/api/studies") - @handle_json_api_exception + @json_api_view def create_study() -> BottleViewReturn: - response.content_type = "application/json" - study_name = request.json.get("study_name", None) directions = request.json.get("directions", []) if ( @@ -191,10 +189,8 @@ def create_app(storage: BaseStorage) -> Bottle: return {"study_summary": serialize_study_summary(summary)} @app.delete("/api/studies/") - @handle_json_api_exception + @json_api_view def delete_study(study_id: int) -> BottleViewReturn: - response.content_type = "application/json" - try: storage.delete_study(study_id) except KeyError: @@ -204,9 +200,8 @@ def create_app(storage: BaseStorage) -> Bottle: return "" @app.get("/api/studies/") - @handle_json_api_exception + @json_api_view def get_study_detail(study_id: int) -> BottleViewReturn: - response.content_type = "application/json" try: after = int(request.params["after"]) assert after >= 0 @@ -230,10 +225,9 @@ def create_app(storage: BaseStorage) -> Bottle: ) @app.get("/api/studies//param_importances") - @handle_json_api_exception + @json_api_view def get_param_importances(study_id: int) -> BottleViewReturn: # TODO(chenghuzi): add support for selecting params via query parameters. - response.content_type = "application/json" objective_id = int(request.params.get("objective_id", 0)) try: study_name = storage.get_study_name_from_id(study_id) @@ -283,10 +277,8 @@ def create_app(storage: BaseStorage) -> Bottle: } @app.put("/api/studies//note") - @handle_json_api_exception + @json_api_view def save_note(study_id: int) -> BottleViewReturn: - response.content_type = "application/json" - req_note_ver = request.json.get("version", None) req_note_body = request.json.get("body", None) if req_note_ver is None or req_note_body is None: From 90922da6628c8bd3e8590d581d2bd8d0009b3a0b Mon Sep 17 00:00:00 2001 From: c-bata Date: Sat, 19 Mar 2022 03:34:23 +0900 Subject: [PATCH 07/12] Make note editor resizable --- optuna_dashboard/static/components/Note.tsx | 3 ++- .../static/components/StudyDetail.tsx | 16 ++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/optuna_dashboard/static/components/Note.tsx b/optuna_dashboard/static/components/Note.tsx index 0ba538b7..5bd5cab1 100644 --- a/optuna_dashboard/static/components/Note.tsx +++ b/optuna_dashboard/static/components/Note.tsx @@ -61,10 +61,11 @@ export const Note: FC<{ { diff --git a/optuna_dashboard/static/components/StudyDetail.tsx b/optuna_dashboard/static/components/StudyDetail.tsx index a18e22df..1715e09d 100644 --- a/optuna_dashboard/static/components/StudyDetail.tsx +++ b/optuna_dashboard/static/components/StudyDetail.tsx @@ -238,14 +238,14 @@ export const StudyDetail: FC<{ /> Editor - } - label="NoteEditor" + control={ + + } + label="NoteEditor" /> From 3a939d419c6ef28af962ccc9d0fba8a378353ee8 Mon Sep 17 00:00:00 2001 From: c-bata Date: Sat, 19 Mar 2022 03:43:23 +0900 Subject: [PATCH 08/12] Enable confirmation dialog when edited --- optuna_dashboard/static/components/Note.tsx | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/optuna_dashboard/static/components/Note.tsx b/optuna_dashboard/static/components/Note.tsx index 5bd5cab1..c920a16c 100644 --- a/optuna_dashboard/static/components/Note.tsx +++ b/optuna_dashboard/static/components/Note.tsx @@ -19,7 +19,7 @@ export const Note: FC<{ }> = ({ studyId, latestNote }) => { const theme = useTheme() const [saving, setSaving] = useState(false) - const [disable, setDisable] = useState(true) + const [edited, setEdited] = useState(false) const [curNote, setCurNote] = useState({ version: 0, body: "" }) const textAreaRef = createRef() const action = actionCreator() @@ -28,6 +28,15 @@ export const Note: FC<{ useEffect(() => { setCurNote(latestNote) }, []) + useEffect(() => { + if (edited) { + window.onbeforeunload = (e) => { + e.returnValue = "Are you okay to discard your changes?" + } + } else { + window.onbeforeunload = null + } + }, [edited]) const handleSave = () => { const nextVersion = curNote.version + 1 const newNote = { @@ -70,7 +79,7 @@ export const Note: FC<{ defaultValue={curNote.body} onChange={() => { const cur = textAreaRef.current ? textAreaRef.current.value : "" - setDisable(cur === curNote.body) + setEdited(cur !== curNote.body) }} /> } variant="contained" - disabled={disable} + disabled={!edited} > Save From af95449d8abaf565c4108d31f8bf69c67da42718 Mon Sep 17 00:00:00 2001 From: c-bata Date: Sat, 19 Mar 2022 04:04:29 +0900 Subject: [PATCH 09/12] Fix lint errors --- frontend_tests/TrialTable.test.tsx | 4 ++++ optuna_dashboard/_app.py | 2 +- optuna_dashboard/_note.py | 10 ++++++++-- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/frontend_tests/TrialTable.test.tsx b/frontend_tests/TrialTable.test.tsx index 7a0fda2d..40935b8a 100644 --- a/frontend_tests/TrialTable.test.tsx +++ b/frontend_tests/TrialTable.test.tsx @@ -74,6 +74,10 @@ const study_detail = { }, ], has_intermediate_values: false, + note: { + version: 0, + body: "", + }, } it("Sort TrialTable by trial number", () => { diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index 66060234..14cb4ae4 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -34,11 +34,11 @@ from optuna.study import StudySummary from optuna.trial import FrozenTrial from optuna.trial import TrialState +from . import _note as note from ._intermediate_values import has_intermediate_values from ._search_space import get_search_space from ._serializer import serialize_study_detail from ._serializer import serialize_study_summary -from . import _note as note if typing.TYPE_CHECKING: diff --git a/optuna_dashboard/_note.py b/optuna_dashboard/_note.py index 8d7fa55b..e7f3b758 100644 --- a/optuna_dashboard/_note.py +++ b/optuna_dashboard/_note.py @@ -1,9 +1,15 @@ -from typing import Dict, Any -from typing import TypedDict import math +from typing import Any +from typing import Dict from optuna.storages import BaseStorage + +try: + from typing import TypedDict +except ImportError: + from typing_extensions import TypedDict + SYSTEM_ATTR_MAX_LENGTH = 2045 NOTE_VER_KEY = "dashboard:note_ver" NOTE_STR_KEY_PREFIX = "dashboard:note_str:" From beb26f90680307d68ccba1a6a92f6613711eb910 Mon Sep 17 00:00:00 2001 From: c-bata Date: Sat, 19 Mar 2022 04:14:49 +0900 Subject: [PATCH 10/12] Fix tests --- frontend_tests/TrialTable.test.tsx | 13 +++++++------ optuna_dashboard/_app.py | 3 ++- optuna_dashboard/_note.py | 9 ++++----- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/frontend_tests/TrialTable.test.tsx b/frontend_tests/TrialTable.test.tsx index 40935b8a..a3bf1e33 100644 --- a/frontend_tests/TrialTable.test.tsx +++ b/frontend_tests/TrialTable.test.tsx @@ -43,7 +43,8 @@ const trials = [ const study_direction: StudyDirection = "minimize" as StudyDirection -const study_detail = { +const studyDetail = { + id: 1, name: "study_0", directions: [study_direction], datetime_start: new Date("2021-06-15T00:00:00"), @@ -82,7 +83,7 @@ const study_detail = { it("Sort TrialTable by trial number", () => { const { getAllByRole, getByText } = render( - + ) const rows = getAllByRole("row") expect(within(rows[1]).getByText("0")).toBeTruthy() @@ -97,7 +98,7 @@ it("Sort TrialTable by trial number", () => { it("Sort TrialTable by value", () => { const { getAllByRole, getByText } = render( - + ) fireEvent.click(getByText("Value")) const rows = getAllByRole("row") @@ -112,7 +113,7 @@ it("Sort TrialTable by value", () => { it("Sort TrialTable by duration", () => { const { getAllByRole, getByText } = render( - + ) fireEvent.click(getByText("Duration(ms)")) const rows = getAllByRole("row") @@ -127,7 +128,7 @@ it("Sort TrialTable by duration", () => { it("Sort TrialTable by state", () => { const { getAllByRole, getByText } = render( - + ) fireEvent.click(getByText("State")) const rows = getAllByRole("row") @@ -141,7 +142,7 @@ it("Sort TrialTable by state", () => { }) it("Filter trials by state", () => { - const { queryAllByText } = render() + const { queryAllByText } = render() expect(queryAllByText("Fail").length).toBe(1) // Click 'Complete' state diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index 14cb4ae4..67a566c5 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -289,7 +289,8 @@ def create_app(storage: BaseStorage) -> Bottle: if not note.version_is_incremented(system_attrs, req_note_ver): response.status = 409 # Conflict return { - "reason": "The text you are editing has changed. Please copy your edits and refresh the page.", + "reason": "The text you are editing has changed. " + "Please copy your edits and refresh the page.", } note.save_note(storage, study_id, req_note_ver, req_note_body) diff --git a/optuna_dashboard/_note.py b/optuna_dashboard/_note.py index e7f3b758..be263d63 100644 --- a/optuna_dashboard/_note.py +++ b/optuna_dashboard/_note.py @@ -35,10 +35,7 @@ def get_note_from_system_attrs(system_attrs: Dict[str, Any]) -> NoteType: for key, value in system_attrs.items() if key.startswith(NOTE_STR_KEY_PREFIX) } - return { - "version": note_ver, - "body": concat_body(note_attrs) - } + return {"version": note_ver, "body": concat_body(note_attrs)} def version_is_incremented(system_attrs: Dict[str, Any], req_note_ver: int) -> bool: @@ -75,4 +72,6 @@ def split_body(note_str: str) -> Dict[str, str]: def concat_body(note_attrs: Dict[str, str]) -> str: - return "".join(note_attrs[f"{NOTE_STR_KEY_PREFIX}{i}"] for i in range(len(note_attrs))) + return "".join( + note_attrs[f"{NOTE_STR_KEY_PREFIX}{i}"] for i in range(len(note_attrs)) + ) From 555cf55519cb1117f12df2a49157aeb5d73972aa Mon Sep 17 00:00:00 2001 From: c-bata Date: Sat, 19 Mar 2022 15:57:18 +0900 Subject: [PATCH 11/12] Return latest note when got 409 status --- optuna_dashboard/_app.py | 1 + optuna_dashboard/static/action.ts | 35 ++++++++++++++------- optuna_dashboard/static/components/Note.tsx | 2 ++ 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index 67a566c5..4158d276 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -291,6 +291,7 @@ def create_app(storage: BaseStorage) -> Bottle: return { "reason": "The text you are editing has changed. " "Please copy your edits and refresh the page.", + "note": note.get_note_from_system_attrs(system_attrs) } note.save_note(storage, study_id, req_note_ver, req_note_body) diff --git a/optuna_dashboard/static/action.ts b/optuna_dashboard/static/action.ts index 76118c3f..e2f86a17 100644 --- a/optuna_dashboard/static/action.ts +++ b/optuna_dashboard/static/action.ts @@ -16,6 +16,12 @@ export const actionCreator = () => { const [studyDetails, setStudyDetails] = useRecoilState(studyDetailsState) + const setStudyDetailState = (studyId: number, study: StudyDetail) => { + const newVal = Object.assign({}, studyDetails) + newVal[studyId] = study + setStudyDetails(newVal) + } + const updateStudySummaries = (successMsg?: string) => { getStudySummariesAPI() .then((studySummaries: StudySummary[]) => { @@ -50,15 +56,15 @@ export const actionCreator = () => { ? studyDetails[studyId].trials.slice(0, nLocalFixedTrials) : [] study.trials = study.trials.concat(currentFixedTrials) - const newVal = Object.assign({}, studyDetails) - newVal[studyId] = study - setStudyDetails(newVal) + setStudyDetailState(studyId, study) }) .catch((err) => { const reason = err.response?.data.reason - enqueueSnackbar(`Failed to fetch study (reason=${reason})`, { - variant: "error", - }) + if (reason !== undefined) { + enqueueSnackbar(`Failed to fetch study (reason=${reason})`, { + variant: "error", + }) + } console.log(err) }) } @@ -101,18 +107,23 @@ export const actionCreator = () => { .then(() => { const newStudy = Object.assign({}, studyDetails[studyId]) newStudy.note = note - const newStudies = Object.assign({}, studyDetails) - newStudies[studyId] = newStudy - setStudyDetails(newStudies) + setStudyDetailState(studyId, newStudy) enqueueSnackbar(`Success to save the note`, { variant: "success", }) }) .catch((err) => { + if (err.response.status === 409) { + const newStudy = Object.assign({}, studyDetails[studyId]) + newStudy.note = err.response.data.note + setStudyDetailState(studyId, newStudy) + } const reason = err.response?.data.reason - enqueueSnackbar(`Failed: ${reason}`, { - variant: "error", - }) + if (reason !== undefined) { + enqueueSnackbar(`Failed: ${reason}`, { + variant: "error", + }) + } throw err }) } diff --git a/optuna_dashboard/static/components/Note.tsx b/optuna_dashboard/static/components/Note.tsx index c920a16c..82176327 100644 --- a/optuna_dashboard/static/components/Note.tsx +++ b/optuna_dashboard/static/components/Note.tsx @@ -48,6 +48,7 @@ export const Note: FC<{ .saveNote(studyId, newNote) .then(() => { setCurNote(newNote) + window.onbeforeunload = null }) .finally(() => { setSaving(false) @@ -60,6 +61,7 @@ export const Note: FC<{ } textAreaRef.current.value = latestNote.body setCurNote(latestNote) + window.onbeforeunload = null } return ( From 6d42be6ada122a0f21faa896c7a4f940721aab78 Mon Sep 17 00:00:00 2001 From: c-bata Date: Sat, 19 Mar 2022 15:59:43 +0900 Subject: [PATCH 12/12] Fix lint errors --- optuna_dashboard/_app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index 4158d276..2bd94c3d 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -291,7 +291,7 @@ def create_app(storage: BaseStorage) -> Bottle: return { "reason": "The text you are editing has changed. " "Please copy your edits and refresh the page.", - "note": note.get_note_from_system_attrs(system_attrs) + "note": note.get_note_from_system_attrs(system_attrs), } note.save_note(storage, study_id, req_note_ver, req_note_body)