From 63ac4996e5120466f3ebaac6e384e41a7712b743 Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Tue, 12 Sep 2023 16:13:51 +0900 Subject: [PATCH 1/3] remove and restore history api --- optuna_dashboard/_app.py | 16 ++++ optuna_dashboard/_preferential_history.py | 34 ++++++--- optuna_dashboard/_serializer.py | 12 ++- .../preferential/_system_attrs.py | 6 ++ python_tests/test_api.py | 60 +++++++++++++++ python_tests/test_preferential_history.py | 75 ++++++++++++++++--- 6 files changed, 173 insertions(+), 30 deletions(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index c32c2061..a3ab595a 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -29,7 +29,9 @@ from ._custom_plot_data import get_plotly_graph_objects from ._importance import get_param_importance_from_trials_cache from ._pareto_front import get_pareto_front_trials from ._preferential_history import NewHistory +from ._preferential_history import remove_history from ._preferential_history import report_history +from ._preferential_history import restore_history from ._rdb_migration import register_rdb_migration_route from ._serializer import serialize_study_detail from ._serializer import serialize_study_summary @@ -306,6 +308,20 @@ def create_app( response.status = 204 return {} + @app.delete("/api/studies//preference/") + @json_api_view + def remove_preference(study_id: int, history_id: str) -> dict[str, Any]: + remove_history(study_id, storage, history_id) + response.status = 204 + return {} + + @app.post("/api/studies//preference/") + @json_api_view + def restore_preference(study_id: int, history_id: str) -> dict[str, Any]: + restore_history(study_id, storage, history_id) + response.status = 204 + return {} + @app.post("/api/trials//tell") @json_api_view def tell_trial(trial_id: int) -> dict[str, Any]: diff --git a/optuna_dashboard/_preferential_history.py b/optuna_dashboard/_preferential_history.py index 6b81b9bb..3d912190 100644 --- a/optuna_dashboard/_preferential_history.py +++ b/optuna_dashboard/_preferential_history.py @@ -4,10 +4,10 @@ from dataclasses import dataclass from datetime import datetime import json from typing import TYPE_CHECKING -import uuid from optuna.storages import BaseStorage +from .preferential._system_attrs import _SYSTEM_ATTR_PREFIX_PREFERENCE from .preferential._system_attrs import report_preferences @@ -23,10 +23,10 @@ if TYPE_CHECKING: { "mode": FeedbackMode, "id": str, - "preference_id": str, "timestamp": str, "candidates": list[int], "clicked": int, + "preferences": list[tuple[int, int]], }, ) History = ChooseWorstHistory @@ -43,38 +43,50 @@ def report_history( study_id: int, storage: BaseStorage, input_data: NewHistory, -) -> None: +) -> str: preferences = [] # TODO(moririn): Use TypeGuard after adding other history types. if input_data.mode == "ChooseWorst": preferences = [ - (best, input_data.clicked) - for best in input_data.candidates - if best != input_data.clicked + (better, input_data.clicked) + for better in input_data.candidates + if better != input_data.clicked ] else: assert False, f"Unknown data: {input_data}" - preference_id = report_preferences( + id = report_preferences( study_id=study_id, storage=storage, preferences=preferences, ) - history_id = str(uuid.uuid4()) if input_data.mode == "ChooseWorst": history: ChooseWorstHistory = { "mode": "ChooseWorst", - "id": history_id, - "preference_id": preference_id, + "id": id, "timestamp": datetime.now().isoformat(), "candidates": input_data.candidates, "clicked": input_data.clicked, + "preferences": preferences, } - key = _SYSTEM_ATTR_PREFIX_HISTORY + history_id + key = _SYSTEM_ATTR_PREFIX_HISTORY + id storage.set_study_system_attr( study_id=study_id, key=key, value=json.dumps(history), ) + return id + + +def remove_history(study_id: int, storage: BaseStorage, id: str) -> None: + storage.set_study_system_attr(study_id, _SYSTEM_ATTR_PREFIX_PREFERENCE + id, []) + + +def restore_history(study_id: int, storage: BaseStorage, id: str) -> None: + system_attrs = storage.get_study_system_attrs(study_id) + history: History = json.loads(system_attrs.get(_SYSTEM_ATTR_PREFIX_HISTORY + id, "")) + storage.set_study_system_attr( + study_id, _SYSTEM_ATTR_PREFIX_PREFERENCE + history["id"], history["preferences"] + ) diff --git a/optuna_dashboard/_serializer.py b/optuna_dashboard/_serializer.py index e3f77649..8388c736 100644 --- a/optuna_dashboard/_serializer.py +++ b/optuna_dashboard/_serializer.py @@ -19,15 +19,13 @@ from ._preferential_history import _SYSTEM_ATTR_PREFIX_HISTORY from .artifact._backend import list_trial_artifacts from .preferential._study import _SYSTEM_ATTR_PREFERENTIAL_STUDY from .preferential._system_attrs import get_preferences +from .preferential._system_attrs import is_preference_removed if TYPE_CHECKING: from typing import Literal from typing import TypedDict - from ._preferential_history import ChooseWorstHistory - from ._preferential_history import History - Attribute = TypedDict( "Attribute", { @@ -174,20 +172,20 @@ def serialize_study_detail( def serialize_preference_history( system_attrs: dict[str, Any], -) -> list[History]: - histories: list[History] = [] +) -> list[dict[str, Any]]: + histories: list[dict[str, Any]] = [] for k, v in system_attrs.items(): if not k.startswith(_SYSTEM_ATTR_PREFIX_HISTORY): continue choice: dict[str, Any] = json.loads(v) if choice["mode"] == "ChooseWorst": - history: ChooseWorstHistory = { + history = { "mode": "ChooseWorst", "id": choice["id"], - "preference_id": choice["preference_id"], "timestamp": choice["timestamp"], "candidates": choice["candidates"], "clicked": choice["clicked"], + "is_removed": is_preference_removed(system_attrs, choice["id"]), } histories.append(history) diff --git a/optuna_dashboard/preferential/_system_attrs.py b/optuna_dashboard/preferential/_system_attrs.py index 47c2a486..33d56f30 100644 --- a/optuna_dashboard/preferential/_system_attrs.py +++ b/optuna_dashboard/preferential/_system_attrs.py @@ -44,6 +44,12 @@ def get_preferences(study_system_attrs: dict[str, Any]) -> list[tuple[int, int]] return preferences +def is_preference_removed(study_system_attrs: dict[str, Any], id: str) -> bool: + key = _SYSTEM_ATTR_PREFIX_PREFERENCE + id + preference = study_system_attrs.get(key, []) + return len(preference) == 0 + + def report_skip( study_id: int, trial_id: int, diff --git a/python_tests/test_api.py b/python_tests/test_api.py index c551e3f2..6075ffa5 100644 --- a/python_tests/test_api.py +++ b/python_tests/test_api.py @@ -8,6 +8,7 @@ from optuna import get_all_study_summaries from optuna.study import StudyDirection from optuna_dashboard._app import create_app from optuna_dashboard._app import create_new_study +from optuna_dashboard._serializer import serialize_preference_history from optuna_dashboard.preferential import create_study from .wsgi_client import send_request @@ -203,6 +204,65 @@ class APITestCase(TestCase): assert len(best_trials) == 1 assert best_trials[0].number == 2 + def test_undo_redo_history(self) -> None: + storage = optuna.storages.InMemoryStorage() + study = create_study(storage=storage, n_generate=3) + for _ in range(3): + study.ask() + + 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": "ChooseWorst", + "candidates": [0, 1, 2], + "clicked": 2, + } + ), + content_type="application/json", + ) + self.assertEqual(status, 204) + histories = serialize_preference_history(storage.get_study_system_attrs(study_id)) + assert len(histories) == 1 + assert not histories[0]["is_removed"] + + history_id = histories[0]["id"] + status, _, _ = send_request( + app, + f"/api/studies/{study_id}/preference/{history_id}", + "DELETE", + content_type="application/json", + ) + self.assertEqual(status, 204) + histories = serialize_preference_history(storage.get_study_system_attrs(study_id)) + assert len(histories) == 1 + assert histories[0]["is_removed"] + assert len(study.get_preferences()) == 0 + + status, _, _ = send_request( + app, + f"/api/studies/{study_id}/preference/{history_id}", + "POST", + content_type="application/json", + ) + self.assertEqual(status, 204) + histories = serialize_preference_history(storage.get_study_system_attrs(study_id)) + assert len(histories) == 1 + assert not histories[0]["is_removed"] + preferences = study.get_preferences() + preferences.sort(key=lambda x: (x[0].number, x[1].number)) + assert len(preferences) == 2 + better, worse = preferences[0] + assert better.number == 0 + assert worse.number == 2 + better, worse = preferences[1] + assert better.number == 1 + assert worse.number == 2 + def test_create_study(self) -> None: for name, directions, expected_status in [ ("single-objective success", ["minimize"], 201), diff --git a/python_tests/test_preferential_history.py b/python_tests/test_preferential_history.py index 51c9b0f8..c64f448c 100644 --- a/python_tests/test_preferential_history.py +++ b/python_tests/test_preferential_history.py @@ -1,9 +1,14 @@ from __future__ import annotations +import json from typing import Callable +from typing import TYPE_CHECKING +from optuna_dashboard._preferential_history import _SYSTEM_ATTR_PREFIX_HISTORY from optuna_dashboard._preferential_history import NewHistory +from optuna_dashboard._preferential_history import remove_history from optuna_dashboard._preferential_history import report_history +from optuna_dashboard._preferential_history import restore_history from optuna_dashboard._serializer import serialize_preference_history from optuna_dashboard.preferential import create_study from optuna_dashboard.preferential._system_attrs import _SYSTEM_ATTR_PREFIX_PREFERENCE @@ -12,6 +17,10 @@ from .storage_supplier import parametrize_storages from .storage_supplier import StorageSupplier +if TYPE_CHECKING: + from optuna_dashboard._preferential_history import History + + @parametrize_storages def test_report_and_get_choices(storage_supplier: Callable[[], StorageSupplier]) -> None: with storage_supplier() as storage: @@ -25,27 +34,19 @@ def test_report_and_get_choices(storage_supplier: Callable[[], StorageSupplier]) report_history( study_id=study_id, storage=storage, - input_data=NewHistory( - mode="ChooseWorst", - candidates=[0, 1, 2], - clicked=1, - ), + input_data=NewHistory(mode="ChooseWorst", candidates=[0, 1, 2], clicked=1), ) report_history( study_id=study_id, storage=storage, - input_data=NewHistory( - mode="ChooseWorst", - candidates=[0, 2, 3, 4], - clicked=0, - ), + input_data=NewHistory(mode="ChooseWorst", candidates=[0, 2, 3, 4], clicked=0), ) history = serialize_preference_history(storage.get_study_system_attrs(study_id)) sys_attrs = storage.get_study_system_attrs(study_id) assert len(history) == 2 assert history[0]["candidates"] == [0, 1, 2] assert history[0]["clicked"] == 1 - preferences = sys_attrs[_SYSTEM_ATTR_PREFIX_PREFERENCE + history[0]["preference_id"]] + preferences = sys_attrs[_SYSTEM_ATTR_PREFIX_PREFERENCE + history[0]["id"]] assert len(preferences) == 2 for i, (best, worst) in enumerate([(0, 1), (2, 1)]): assert len(preferences[i]) == 2 @@ -53,9 +54,59 @@ def test_report_and_get_choices(storage_supplier: Callable[[], StorageSupplier]) assert preferences[i][1] == worst assert history[1]["candidates"] == [0, 2, 3, 4] assert history[1]["clicked"] == 0 - preferences = sys_attrs[_SYSTEM_ATTR_PREFIX_PREFERENCE + history[1]["preference_id"]] + preferences = sys_attrs[_SYSTEM_ATTR_PREFIX_PREFERENCE + history[1]["id"]] assert len(preferences) == 3 for i, (best, worst) in enumerate([(2, 0), (3, 0), (4, 0)]): assert len(preferences[i]) == 2 assert preferences[i][0] == best assert preferences[i][1] == worst + + +@parametrize_storages +def test_undo_redo_history(storage_supplier: Callable[[], StorageSupplier]) -> None: + with storage_supplier() as storage: + study = create_study(storage=storage, n_generate=5) + for _ in range(5): + trial = study.ask() + trial.suggest_float("x", 0, 1) + + study_id = study._study._study_id + + def get_preferences_history(id: str) -> tuple[list[tuple[int, int]], History]: + system_attrs = storage.get_study_system_attrs(study_id) + history: History = json.loads(system_attrs.get(_SYSTEM_ATTR_PREFIX_HISTORY + id, "")) + preference: list[tuple[int, int]] = system_attrs.get( + _SYSTEM_ATTR_PREFIX_PREFERENCE + id, [] + ) + return preference, history + + history_id = report_history( + study_id=study_id, + storage=storage, + input_data=NewHistory(mode="ChooseWorst", candidates=[0, 1, 2], clicked=1), + ) + remove_history(study_id, storage, history_id) + preference, history = get_preferences_history(history_id) + assert history["mode"] == "ChooseWorst" + assert history["candidates"] == [0, 1, 2] + assert history["clicked"] == 1 + assert len(preference) == 0 + + remove_history(study_id, storage, history_id) + preference, history = get_preferences_history(history_id) + assert len(preference) == 0 + + restore_history(study_id, storage, history_id) + preference, history = get_preferences_history(history_id) + assert history["mode"] == "ChooseWorst" + assert history["candidates"] == [0, 1, 2] + assert history["clicked"] == 1 + assert len(preference) == 2 + for i, (best, worst) in enumerate([(0, 1), (2, 1)]): + assert len(preference[i]) == 2 + assert preference[i][0] == best + assert preference[i][1] == worst + + restore_history(study_id, storage, history_id) + preference, history = get_preferences_history(history_id) + assert len(preference) == 2 From ba82fadad9d735684c9cf3879951407a835c6599 Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Tue, 12 Sep 2023 17:43:23 +0900 Subject: [PATCH 2/3] fix by review --- optuna_dashboard/_app.py | 15 ++++- optuna_dashboard/_preferential_history.py | 32 ++++++++-- optuna_dashboard/_serializer.py | 20 ++++-- .../preferential/_system_attrs.py | 4 +- python_tests/test_api.py | 48 ++++++++++---- python_tests/test_preferential_history.py | 64 +++++++++++++------ 6 files changed, 132 insertions(+), 51 deletions(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index a3ab595a..a020c8b8 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -28,6 +28,7 @@ from ._cached_extra_study_property import get_cached_extra_study_property from ._custom_plot_data import get_plotly_graph_objects from ._importance import get_param_importance_from_trials_cache from ._pareto_front import get_pareto_front_trials +from ._preferential_history import HistoryIdError from ._preferential_history import NewHistory from ._preferential_history import remove_history from ._preferential_history import report_history @@ -311,14 +312,24 @@ def create_app( @app.delete("/api/studies//preference/") @json_api_view def remove_preference(study_id: int, history_id: str) -> dict[str, Any]: - remove_history(study_id, storage, history_id) + try: + remove_history(study_id, storage, history_id) + except HistoryIdError: + response.status = 404 + return {"reason": f"history_id={history_id} is not found"} + response.status = 204 return {} @app.post("/api/studies//preference/") @json_api_view def restore_preference(study_id: int, history_id: str) -> dict[str, Any]: - restore_history(study_id, storage, history_id) + try: + restore_history(study_id, storage, history_id) + except HistoryIdError: + response.status = 404 + return {"reason": f"history_id={history_id} is not found"} + response.status = 204 return {} diff --git a/optuna_dashboard/_preferential_history.py b/optuna_dashboard/_preferential_history.py index 3d912190..9726d27c 100644 --- a/optuna_dashboard/_preferential_history.py +++ b/optuna_dashboard/_preferential_history.py @@ -30,6 +30,17 @@ if TYPE_CHECKING: }, ) History = ChooseWorstHistory + SerializedHistory = TypedDict( + "SerializedHistory", + { + "history": History, + "is_removed": bool, + }, + ) + + +class HistoryIdError(Exception): + pass @dataclass @@ -80,13 +91,20 @@ def report_history( return id -def remove_history(study_id: int, storage: BaseStorage, id: str) -> None: - storage.set_study_system_attr(study_id, _SYSTEM_ATTR_PREFIX_PREFERENCE + id, []) - - -def restore_history(study_id: int, storage: BaseStorage, id: str) -> None: +def remove_history(study_id: int, storage: BaseStorage, history_id: str) -> None: system_attrs = storage.get_study_system_attrs(study_id) - history: History = json.loads(system_attrs.get(_SYSTEM_ATTR_PREFIX_HISTORY + id, "")) + history_key = _SYSTEM_ATTR_PREFIX_HISTORY + history_id + if history_key not in system_attrs: + raise HistoryIdError + storage.set_study_system_attr(study_id, _SYSTEM_ATTR_PREFIX_PREFERENCE + history_id, []) + + +def restore_history(study_id: int, storage: BaseStorage, history_id: str) -> None: + system_attrs = storage.get_study_system_attrs(study_id) + history_key = _SYSTEM_ATTR_PREFIX_HISTORY + history_id + if history_key not in system_attrs: + raise HistoryIdError + history: History = json.loads(system_attrs.get(history_key, "")) storage.set_study_system_attr( - study_id, _SYSTEM_ATTR_PREFIX_PREFERENCE + history["id"], history["preferences"] + study_id, _SYSTEM_ATTR_PREFIX_PREFERENCE + history_id, history["preferences"] ) diff --git a/optuna_dashboard/_serializer.py b/optuna_dashboard/_serializer.py index 8388c736..001f689a 100644 --- a/optuna_dashboard/_serializer.py +++ b/optuna_dashboard/_serializer.py @@ -26,6 +26,9 @@ if TYPE_CHECKING: from typing import Literal from typing import TypedDict + from ._preferential_history import History + from ._preferential_history import SerializedHistory + Attribute = TypedDict( "Attribute", { @@ -172,24 +175,29 @@ def serialize_study_detail( def serialize_preference_history( system_attrs: dict[str, Any], -) -> list[dict[str, Any]]: - histories: list[dict[str, Any]] = [] +) -> list[SerializedHistory]: + histories: list[SerializedHistory] = [] for k, v in system_attrs.items(): if not k.startswith(_SYSTEM_ATTR_PREFIX_HISTORY): continue choice: dict[str, Any] = json.loads(v) if choice["mode"] == "ChooseWorst": - history = { + history: History = { "mode": "ChooseWorst", "id": choice["id"], "timestamp": choice["timestamp"], "candidates": choice["candidates"], "clicked": choice["clicked"], - "is_removed": is_preference_removed(system_attrs, choice["id"]), + "preferences": choice["preferences"], } - histories.append(history) + histories.append( + { + "history": history, + "is_removed": is_preference_removed(system_attrs, choice["id"]), + } + ) - histories.sort(key=lambda c: datetime.fromisoformat(c["timestamp"])) + histories.sort(key=lambda c: datetime.fromisoformat(c["history"]["timestamp"])) return histories diff --git a/optuna_dashboard/preferential/_system_attrs.py b/optuna_dashboard/preferential/_system_attrs.py index 33d56f30..438b411c 100644 --- a/optuna_dashboard/preferential/_system_attrs.py +++ b/optuna_dashboard/preferential/_system_attrs.py @@ -44,8 +44,8 @@ def get_preferences(study_system_attrs: dict[str, Any]) -> list[tuple[int, int]] return preferences -def is_preference_removed(study_system_attrs: dict[str, Any], id: str) -> bool: - key = _SYSTEM_ATTR_PREFIX_PREFERENCE + id +def is_preference_removed(study_system_attrs: dict[str, Any], preference_id: str) -> bool: + key = _SYSTEM_ATTR_PREFIX_PREFERENCE + preference_id preference = study_system_attrs.get(key, []) return len(preference) == 0 diff --git a/python_tests/test_api.py b/python_tests/test_api.py index 6075ffa5..afe5a31c 100644 --- a/python_tests/test_api.py +++ b/python_tests/test_api.py @@ -8,6 +8,9 @@ from optuna import get_all_study_summaries from optuna.study import StudyDirection from optuna_dashboard._app import create_app from optuna_dashboard._app import create_new_study +from optuna_dashboard._preferential_history import NewHistory +from optuna_dashboard._preferential_history import remove_history +from optuna_dashboard._preferential_history import report_history from optuna_dashboard._serializer import serialize_preference_history from optuna_dashboard.preferential import create_study @@ -204,7 +207,7 @@ class APITestCase(TestCase): assert len(best_trials) == 1 assert best_trials[0].number == 2 - def test_undo_redo_history(self) -> None: + def test_remove_history(self) -> None: storage = optuna.storages.InMemoryStorage() study = create_study(storage=storage, n_generate=3) for _ in range(3): @@ -212,25 +215,19 @@ class APITestCase(TestCase): 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": "ChooseWorst", - "candidates": [0, 1, 2], - "clicked": 2, - } + history_id = report_history( + study_id, + storage, + NewHistory( + mode="ChooseWorst", + candidates=[0, 1, 2], + clicked=2, ), - content_type="application/json", ) - self.assertEqual(status, 204) histories = serialize_preference_history(storage.get_study_system_attrs(study_id)) assert len(histories) == 1 assert not histories[0]["is_removed"] - history_id = histories[0]["id"] status, _, _ = send_request( app, f"/api/studies/{study_id}/preference/{history_id}", @@ -243,6 +240,29 @@ class APITestCase(TestCase): assert histories[0]["is_removed"] assert len(study.get_preferences()) == 0 + def test_restore_history(self) -> None: + storage = optuna.storages.InMemoryStorage() + study = create_study(storage=storage, n_generate=3) + for _ in range(3): + study.ask() + + app = create_app(storage) + study_id = study._study._study_id + history_id = report_history( + study_id, + storage, + NewHistory( + mode="ChooseWorst", + candidates=[0, 1, 2], + clicked=2, + ), + ) + remove_history(study_id, storage, history_id) + histories = serialize_preference_history(storage.get_study_system_attrs(study_id)) + assert len(histories) == 1 + assert histories[0]["is_removed"] + assert len(study.get_preferences()) == 0 + status, _, _ = send_request( app, f"/api/studies/{study_id}/preference/{history_id}", diff --git a/python_tests/test_preferential_history.py b/python_tests/test_preferential_history.py index c64f448c..3d1a7d70 100644 --- a/python_tests/test_preferential_history.py +++ b/python_tests/test_preferential_history.py @@ -4,6 +4,7 @@ import json from typing import Callable from typing import TYPE_CHECKING +from optuna.storages import BaseStorage from optuna_dashboard._preferential_history import _SYSTEM_ATTR_PREFIX_HISTORY from optuna_dashboard._preferential_history import NewHistory from optuna_dashboard._preferential_history import remove_history @@ -44,17 +45,17 @@ def test_report_and_get_choices(storage_supplier: Callable[[], StorageSupplier]) history = serialize_preference_history(storage.get_study_system_attrs(study_id)) sys_attrs = storage.get_study_system_attrs(study_id) assert len(history) == 2 - assert history[0]["candidates"] == [0, 1, 2] - assert history[0]["clicked"] == 1 - preferences = sys_attrs[_SYSTEM_ATTR_PREFIX_PREFERENCE + history[0]["id"]] + assert history[0]["history"]["candidates"] == [0, 1, 2] + assert history[0]["history"]["clicked"] == 1 + preferences = sys_attrs[_SYSTEM_ATTR_PREFIX_PREFERENCE + history[0]["history"]["id"]] assert len(preferences) == 2 for i, (best, worst) in enumerate([(0, 1), (2, 1)]): assert len(preferences[i]) == 2 assert preferences[i][0] == best assert preferences[i][1] == worst - assert history[1]["candidates"] == [0, 2, 3, 4] - assert history[1]["clicked"] == 0 - preferences = sys_attrs[_SYSTEM_ATTR_PREFIX_PREFERENCE + history[1]["id"]] + assert history[1]["history"]["candidates"] == [0, 2, 3, 4] + assert history[1]["history"]["clicked"] == 0 + preferences = sys_attrs[_SYSTEM_ATTR_PREFIX_PREFERENCE + history[1]["history"]["id"]] assert len(preferences) == 3 for i, (best, worst) in enumerate([(2, 0), (3, 0), (4, 0)]): assert len(preferences[i]) == 2 @@ -62,42 +63,65 @@ def test_report_and_get_choices(storage_supplier: Callable[[], StorageSupplier]) assert preferences[i][1] == worst +def get_preferences_history( + study_id: int, + storage: BaseStorage, + history_id: str, +) -> tuple[list[tuple[int, int]], History]: + system_attrs = storage.get_study_system_attrs(study_id) + history: History = json.loads(system_attrs.get(_SYSTEM_ATTR_PREFIX_HISTORY + history_id, "")) + preference: list[tuple[int, int]] = system_attrs.get( + _SYSTEM_ATTR_PREFIX_PREFERENCE + history_id, [] + ) + return preference, history + + @parametrize_storages -def test_undo_redo_history(storage_supplier: Callable[[], StorageSupplier]) -> None: +def test_remove_history(storage_supplier: Callable[[], StorageSupplier]) -> None: with storage_supplier() as storage: study = create_study(storage=storage, n_generate=5) for _ in range(5): trial = study.ask() trial.suggest_float("x", 0, 1) - study_id = study._study._study_id - def get_preferences_history(id: str) -> tuple[list[tuple[int, int]], History]: - system_attrs = storage.get_study_system_attrs(study_id) - history: History = json.loads(system_attrs.get(_SYSTEM_ATTR_PREFIX_HISTORY + id, "")) - preference: list[tuple[int, int]] = system_attrs.get( - _SYSTEM_ATTR_PREFIX_PREFERENCE + id, [] - ) - return preference, history - history_id = report_history( study_id=study_id, storage=storage, input_data=NewHistory(mode="ChooseWorst", candidates=[0, 1, 2], clicked=1), ) remove_history(study_id, storage, history_id) - preference, history = get_preferences_history(history_id) + preference, history = get_preferences_history(study_id, storage, history_id) assert history["mode"] == "ChooseWorst" assert history["candidates"] == [0, 1, 2] assert history["clicked"] == 1 assert len(preference) == 0 remove_history(study_id, storage, history_id) - preference, history = get_preferences_history(history_id) + preference, history = get_preferences_history(study_id, storage, history_id) + assert len(preference) == 0 + + +@parametrize_storages +def test_restore_history(storage_supplier: Callable[[], StorageSupplier]) -> None: + with storage_supplier() as storage: + study = create_study(storage=storage, n_generate=5) + for _ in range(5): + trial = study.ask() + trial.suggest_float("x", 0, 1) + study_id = study._study._study_id + + history_id = report_history( + study_id=study_id, + storage=storage, + input_data=NewHistory(mode="ChooseWorst", candidates=[0, 1, 2], clicked=1), + ) + remove_history(study_id, storage, history_id) + preference, history = get_preferences_history(study_id, storage, history_id) assert len(preference) == 0 restore_history(study_id, storage, history_id) - preference, history = get_preferences_history(history_id) + preference, history = get_preferences_history(study_id, storage, history_id) assert history["mode"] == "ChooseWorst" assert history["candidates"] == [0, 1, 2] assert history["clicked"] == 1 @@ -108,5 +132,5 @@ def test_undo_redo_history(storage_supplier: Callable[[], StorageSupplier]) -> N assert preference[i][1] == worst restore_history(study_id, storage, history_id) - preference, history = get_preferences_history(history_id) + preference, history = get_preferences_history(study_id, storage, history_id) assert len(preference) == 2 From 53794eac73cf8029569811a17e254b6f28646bcc Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Tue, 12 Sep 2023 18:05:51 +0900 Subject: [PATCH 3/3] fix by review --- optuna_dashboard/_app.py | 6 +++--- optuna_dashboard/_preferential_history.py | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index a020c8b8..0249de48 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -28,8 +28,8 @@ from ._cached_extra_study_property import get_cached_extra_study_property from ._custom_plot_data import get_plotly_graph_objects from ._importance import get_param_importance_from_trials_cache from ._pareto_front import get_pareto_front_trials -from ._preferential_history import HistoryIdError from ._preferential_history import NewHistory +from ._preferential_history import PreferenceHistoryNotFound from ._preferential_history import remove_history from ._preferential_history import report_history from ._preferential_history import restore_history @@ -314,7 +314,7 @@ def create_app( def remove_preference(study_id: int, history_id: str) -> dict[str, Any]: try: remove_history(study_id, storage, history_id) - except HistoryIdError: + except PreferenceHistoryNotFound: response.status = 404 return {"reason": f"history_id={history_id} is not found"} @@ -326,7 +326,7 @@ def create_app( def restore_preference(study_id: int, history_id: str) -> dict[str, Any]: try: restore_history(study_id, storage, history_id) - except HistoryIdError: + except PreferenceHistoryNotFound: response.status = 404 return {"reason": f"history_id={history_id} is not found"} diff --git a/optuna_dashboard/_preferential_history.py b/optuna_dashboard/_preferential_history.py index 9726d27c..ef3f87f4 100644 --- a/optuna_dashboard/_preferential_history.py +++ b/optuna_dashboard/_preferential_history.py @@ -39,7 +39,7 @@ if TYPE_CHECKING: ) -class HistoryIdError(Exception): +class PreferenceHistoryNotFound(Exception): pass @@ -66,7 +66,7 @@ def report_history( else: assert False, f"Unknown data: {input_data}" - id = report_preferences( + preference_id = report_preferences( study_id=study_id, storage=storage, preferences=preferences, @@ -75,27 +75,27 @@ def report_history( if input_data.mode == "ChooseWorst": history: ChooseWorstHistory = { "mode": "ChooseWorst", - "id": id, + "id": preference_id, "timestamp": datetime.now().isoformat(), "candidates": input_data.candidates, "clicked": input_data.clicked, "preferences": preferences, } - key = _SYSTEM_ATTR_PREFIX_HISTORY + id + key = _SYSTEM_ATTR_PREFIX_HISTORY + preference_id storage.set_study_system_attr( study_id=study_id, key=key, value=json.dumps(history), ) - return id + return preference_id def remove_history(study_id: int, storage: BaseStorage, history_id: str) -> None: system_attrs = storage.get_study_system_attrs(study_id) history_key = _SYSTEM_ATTR_PREFIX_HISTORY + history_id if history_key not in system_attrs: - raise HistoryIdError + raise PreferenceHistoryNotFound storage.set_study_system_attr(study_id, _SYSTEM_ATTR_PREFIX_PREFERENCE + history_id, []) @@ -103,7 +103,7 @@ def restore_history(study_id: int, storage: BaseStorage, history_id: str) -> Non system_attrs = storage.get_study_system_attrs(study_id) history_key = _SYSTEM_ATTR_PREFIX_HISTORY + history_id if history_key not in system_attrs: - raise HistoryIdError + raise PreferenceHistoryNotFound history: History = json.loads(system_attrs.get(history_key, "")) storage.set_study_system_attr( study_id, _SYSTEM_ATTR_PREFIX_PREFERENCE + history_id, history["preferences"]