From c30fe8234f7ddb2e3206d6fa295a0d174dc9f15d Mon Sep 17 00:00:00 2001 From: gen740 Date: Mon, 28 Nov 2022 17:46:58 +0900 Subject: [PATCH 1/7] Interface change for `set_study_directions` --- optuna_dashboard/_app.py | 16 +++++++--------- python_tests/test_api.py | 11 ++++++----- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index caf2be02..65a5bb00 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -249,19 +249,17 @@ def create_app(storage: BaseStorage, debug: bool = False) -> Bottle: return {"reason": "You need to set study_name and direction"} try: - study_id = storage.create_new_study(study_name) + study_id = storage.create_new_study( + study_name, + directions=[ + StudyDirection.MAXIMIZE if d.lower() == "maximize" else StudyDirection.MINIMIZE + for d in directions + ], + ) except DuplicatedStudyError: response.status = 400 # Bad request return {"reason": f"'{study_name}' is already exists"} - storage.set_study_directions( - study_id, - [ - StudyDirection.MAXIMIZE if d.lower() == "maximize" else StudyDirection.MINIMIZE - for d in directions - ], - ) - summary = get_study_summary(storage, study_id) if summary is None: response.status = 500 # Internal server error diff --git a/python_tests/test_api.py b/python_tests/test_api.py index 5a795c71..ead1ffc0 100644 --- a/python_tests/test_api.py +++ b/python_tests/test_api.py @@ -2,6 +2,7 @@ import json from unittest import TestCase import optuna +from optuna.study import StudyDirection from optuna import get_all_study_summaries from optuna_dashboard._app import create_app @@ -16,8 +17,8 @@ def objective(trial: optuna.trial.Trial) -> float: class APITestCase(TestCase): def test_get_study_summaries(self) -> None: storage = optuna.storages.InMemoryStorage() - storage.create_new_study("foo1") - storage.create_new_study("foo2") + storage.create_new_study(study_name="foo1", directions=[StudyDirection.MINIMIZE]) + storage.create_new_study(study_name="foo2", directions=[StudyDirection.MINIMIZE]) app = create_app(storage) status, _, body = send_request( @@ -126,7 +127,7 @@ class APITestCase(TestCase): def test_create_study_duplicated(self) -> None: storage = optuna.storages.InMemoryStorage() - storage.create_new_study("foo") + storage.create_new_study(study_name="foo", directions=[StudyDirection.MINIMIZE]) self.assertEqual(len(get_all_study_summaries(storage)), 1) app = create_app(storage) @@ -146,8 +147,8 @@ class APITestCase(TestCase): def test_delete_study(self) -> None: storage = optuna.storages.InMemoryStorage() - storage.create_new_study("foo1") - storage.create_new_study("foo2") + storage.create_new_study(study_name="foo1", directions=[StudyDirection.MINIMIZE]) + storage.create_new_study(study_name="foo2", directions=[StudyDirection.MINIMIZE]) self.assertEqual(len(get_all_study_summaries(storage)), 2) app = create_app(storage) From 91a8de1d55eb2a7a956285791d0277c5fa3cb353 Mon Sep 17 00:00:00 2001 From: gen740 Date: Wed, 30 Nov 2022 16:09:03 +0900 Subject: [PATCH 2/7] Support for backward compatibility. --- optuna_dashboard/_app.py | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index 65a5bb00..8d9c366f 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -249,13 +249,33 @@ def create_app(storage: BaseStorage, debug: bool = False) -> Bottle: return {"reason": "You need to set study_name and direction"} try: - study_id = storage.create_new_study( - study_name, - directions=[ - StudyDirection.MAXIMIZE if d.lower() == "maximize" else StudyDirection.MINIMIZE - for d in directions - ], - ) + # if version.parse(optuna_ver) >= version.Version("3.0.0rc0.dev"): + # frozen_studies = storage.get_all_studies() # type: ignore + # return [_frozen_study_to_study_summary(s) for s in frozen_studies] + # elif version.parse(optuna_ver) >= version.Version("3.0.0b0.dev"): + # return storage.get_all_study_summaries(include_best_trial=False) # type: ignore + if version.parse(optuna_ver) >= version.Version("3.1.0.dev"): + study_id = storage.create_new_study( + study_name, + directions=[ + StudyDirection.MAXIMIZE + if d.lower() == "maximize" + else StudyDirection.MINIMIZE + for d in directions + ], + ) + else: + study_id = storage.create_new_study(study_name) + storage.set_study_directions( + study_id, + [ + StudyDirection.MAXIMIZE + if d.lower() == "maximize" + else StudyDirection.MINIMIZE + for d in directions + ], + ) + except DuplicatedStudyError: response.status = 400 # Bad request return {"reason": f"'{study_name}' is already exists"} From 188b88f26bd564ef6aba68e0d64e5341bd6dc44d Mon Sep 17 00:00:00 2001 From: gen740 Date: Thu, 1 Dec 2022 12:26:21 +0900 Subject: [PATCH 3/7] Fixed tests --- optuna_dashboard/_app.py | 5 ----- python_tests/test_api.py | 26 ++++++++++++++++++++------ 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index 8d9c366f..9a2a514e 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -249,11 +249,6 @@ def create_app(storage: BaseStorage, debug: bool = False) -> Bottle: return {"reason": "You need to set study_name and direction"} try: - # if version.parse(optuna_ver) >= version.Version("3.0.0rc0.dev"): - # frozen_studies = storage.get_all_studies() # type: ignore - # return [_frozen_study_to_study_summary(s) for s in frozen_studies] - # elif version.parse(optuna_ver) >= version.Version("3.0.0b0.dev"): - # return storage.get_all_study_summaries(include_best_trial=False) # type: ignore if version.parse(optuna_ver) >= version.Version("3.1.0.dev"): study_id = storage.create_new_study( study_name, diff --git a/python_tests/test_api.py b/python_tests/test_api.py index ead1ffc0..0bc8ee9f 100644 --- a/python_tests/test_api.py +++ b/python_tests/test_api.py @@ -2,9 +2,11 @@ import json from unittest import TestCase import optuna -from optuna.study import StudyDirection from optuna import get_all_study_summaries +from optuna.study import StudyDirection +from optuna.version import __version__ as optuna_ver from optuna_dashboard._app import create_app +from packaging import version from .wsgi_client import send_request @@ -17,8 +19,13 @@ def objective(trial: optuna.trial.Trial) -> float: class APITestCase(TestCase): def test_get_study_summaries(self) -> None: storage = optuna.storages.InMemoryStorage() - storage.create_new_study(study_name="foo1", directions=[StudyDirection.MINIMIZE]) - storage.create_new_study(study_name="foo2", directions=[StudyDirection.MINIMIZE]) + + if version.parse(optuna_ver) >= version.Version("3.1.0.dev"): + storage.create_new_study(study_name="foo1", directions=[StudyDirection.MINIMIZE]) + storage.create_new_study(study_name="foo2", directions=[StudyDirection.MINIMIZE]) + else: + storage.create_new_study("foo1") + storage.create_new_study("foo2") app = create_app(storage) status, _, body = send_request( @@ -127,7 +134,10 @@ class APITestCase(TestCase): def test_create_study_duplicated(self) -> None: storage = optuna.storages.InMemoryStorage() - storage.create_new_study(study_name="foo", directions=[StudyDirection.MINIMIZE]) + if version.parse(optuna_ver) >= version.Version("3.1.0.dev"): + storage.create_new_study(study_name="foo", directions=[StudyDirection.MINIMIZE]) + else: + storage.create_new_study("foo") self.assertEqual(len(get_all_study_summaries(storage)), 1) app = create_app(storage) @@ -147,8 +157,12 @@ class APITestCase(TestCase): def test_delete_study(self) -> None: storage = optuna.storages.InMemoryStorage() - storage.create_new_study(study_name="foo1", directions=[StudyDirection.MINIMIZE]) - storage.create_new_study(study_name="foo2", directions=[StudyDirection.MINIMIZE]) + if version.parse(optuna_ver) >= version.Version("3.1.0.dev"): + storage.create_new_study(study_name="foo1", directions=[StudyDirection.MINIMIZE]) + storage.create_new_study(study_name="foo2", directions=[StudyDirection.MINIMIZE]) + else: + storage.create_new_study(study_name="foo1") + storage.create_new_study(study_name="foo2") self.assertEqual(len(get_all_study_summaries(storage)), 2) app = create_app(storage) From abefb6d030cb476cd85842b3a9b2eb7245d72279 Mon Sep 17 00:00:00 2001 From: gen740 Date: Thu, 1 Dec 2022 12:36:34 +0900 Subject: [PATCH 4/7] Add `type: ignore` comment --- optuna_dashboard/_app.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index 9a2a514e..15b46456 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -258,7 +258,7 @@ def create_app(storage: BaseStorage, debug: bool = False) -> Bottle: else StudyDirection.MINIMIZE for d in directions ], - ) + ) # type: ignore else: study_id = storage.create_new_study(study_name) storage.set_study_directions( @@ -269,8 +269,7 @@ def create_app(storage: BaseStorage, debug: bool = False) -> Bottle: else StudyDirection.MINIMIZE for d in directions ], - ) - + ) # type: ignore except DuplicatedStudyError: response.status = 400 # Bad request return {"reason": f"'{study_name}' is already exists"} From 83e8aec664375a6d1ec0b0fed14702bb60aa2e65 Mon Sep 17 00:00:00 2001 From: gen740 Date: Thu, 1 Dec 2022 15:11:26 +0900 Subject: [PATCH 5/7] Add `# type: ignore` to `python_tests/test_api.py`. --- python_tests/test_api.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/python_tests/test_api.py b/python_tests/test_api.py index 0bc8ee9f..4f12a2e3 100644 --- a/python_tests/test_api.py +++ b/python_tests/test_api.py @@ -21,8 +21,12 @@ class APITestCase(TestCase): storage = optuna.storages.InMemoryStorage() if version.parse(optuna_ver) >= version.Version("3.1.0.dev"): - storage.create_new_study(study_name="foo1", directions=[StudyDirection.MINIMIZE]) - storage.create_new_study(study_name="foo2", directions=[StudyDirection.MINIMIZE]) + storage.create_new_study( + study_name="foo1", directions=[StudyDirection.MINIMIZE] + ) # type: ignore + storage.create_new_study( + study_name="foo2", directions=[StudyDirection.MINIMIZE] + ) # type: ignore else: storage.create_new_study("foo1") storage.create_new_study("foo2") @@ -135,7 +139,9 @@ class APITestCase(TestCase): def test_create_study_duplicated(self) -> None: storage = optuna.storages.InMemoryStorage() if version.parse(optuna_ver) >= version.Version("3.1.0.dev"): - storage.create_new_study(study_name="foo", directions=[StudyDirection.MINIMIZE]) + storage.create_new_study( + study_name="foo", directions=[StudyDirection.MINIMIZE] + ) # type: ignore else: storage.create_new_study("foo") self.assertEqual(len(get_all_study_summaries(storage)), 1) @@ -158,8 +164,12 @@ class APITestCase(TestCase): def test_delete_study(self) -> None: storage = optuna.storages.InMemoryStorage() if version.parse(optuna_ver) >= version.Version("3.1.0.dev"): - storage.create_new_study(study_name="foo1", directions=[StudyDirection.MINIMIZE]) - storage.create_new_study(study_name="foo2", directions=[StudyDirection.MINIMIZE]) + storage.create_new_study( + study_name="foo1", directions=[StudyDirection.MINIMIZE] + ) # type: ignore + storage.create_new_study( + study_name="foo2", directions=[StudyDirection.MINIMIZE] + ) # type: ignore else: storage.create_new_study(study_name="foo1") storage.create_new_study(study_name="foo2") From be63f0a32b6aa5a5b8bef81c6b229d547951d377 Mon Sep 17 00:00:00 2001 From: c-bata Date: Thu, 22 Dec 2022 23:00:59 +0900 Subject: [PATCH 6/7] Add follow-up changes for #292 --- optuna_dashboard/_app.py | 40 ++++++++++++++++++---------------------- python_tests/test_api.py | 35 ++++++----------------------------- 2 files changed, 24 insertions(+), 51 deletions(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index af9f3841..461da83d 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -159,6 +159,19 @@ def get_study_summary(storage: BaseStorage, study_id: int) -> Optional[StudySumm return None +def create_new_study( + storage: BaseStorage, study_name: str, directions: List[StudyDirection] +) -> int: + if version.parse(optuna_ver) >= version.Version("3.1.0.dev") and version.parse( + optuna_ver + ) != version.Version("3.1.0b0"): + study_id = storage.create_new_study(study_name, directions=directions) # type: ignore + else: + study_id = storage.create_new_study(study_name) + storage.set_study_directions(study_id, directions) # type: ignore + return study_id + + def get_trials(storage: BaseStorage, study_id: int, ttl_seconds: int = 10) -> List[FrozenTrial]: with trials_cache_lock: trials = trials_cache.get(study_id, None) @@ -241,7 +254,10 @@ def create_app(storage: BaseStorage, debug: bool = False) -> Bottle: @json_api_view def create_study() -> BottleViewReturn: study_name = request.json.get("study_name", None) - directions = request.json.get("directions", []) + directions = [ + StudyDirection.MAXIMIZE if d.lower() == "maximize" else StudyDirection.MINIMIZE + for d in request.json.get("directions", []) + ] if ( study_name is None or len(directions) == 0 @@ -251,27 +267,7 @@ def create_app(storage: BaseStorage, debug: bool = False) -> Bottle: return {"reason": "You need to set study_name and direction"} try: - if version.parse(optuna_ver) >= version.Version("3.1.0.dev"): - study_id = storage.create_new_study( - study_name, - directions=[ - StudyDirection.MAXIMIZE - if d.lower() == "maximize" - else StudyDirection.MINIMIZE - for d in directions - ], - ) # type: ignore - else: - study_id = storage.create_new_study(study_name) - storage.set_study_directions( - study_id, - [ - StudyDirection.MAXIMIZE - if d.lower() == "maximize" - else StudyDirection.MINIMIZE - for d in directions - ], - ) # type: ignore + study_id = create_new_study(storage, study_name, directions) except DuplicatedStudyError: response.status = 400 # Bad request return {"reason": f"'{study_name}' already exists"} diff --git a/python_tests/test_api.py b/python_tests/test_api.py index 4f12a2e3..ceb8f60a 100644 --- a/python_tests/test_api.py +++ b/python_tests/test_api.py @@ -4,9 +4,8 @@ from unittest import TestCase import optuna from optuna import get_all_study_summaries from optuna.study import StudyDirection -from optuna.version import __version__ as optuna_ver from optuna_dashboard._app import create_app -from packaging import version +from optuna_dashboard._app import create_new_study from .wsgi_client import send_request @@ -19,17 +18,8 @@ def objective(trial: optuna.trial.Trial) -> float: class APITestCase(TestCase): def test_get_study_summaries(self) -> None: storage = optuna.storages.InMemoryStorage() - - if version.parse(optuna_ver) >= version.Version("3.1.0.dev"): - storage.create_new_study( - study_name="foo1", directions=[StudyDirection.MINIMIZE] - ) # type: ignore - storage.create_new_study( - study_name="foo2", directions=[StudyDirection.MINIMIZE] - ) # type: ignore - else: - storage.create_new_study("foo1") - storage.create_new_study("foo2") + create_new_study(storage, "foo1", [StudyDirection.MINIMIZE]) + create_new_study(storage, "foo2", [StudyDirection.MINIMIZE]) app = create_app(storage) status, _, body = send_request( @@ -138,12 +128,7 @@ class APITestCase(TestCase): def test_create_study_duplicated(self) -> None: storage = optuna.storages.InMemoryStorage() - if version.parse(optuna_ver) >= version.Version("3.1.0.dev"): - storage.create_new_study( - study_name="foo", directions=[StudyDirection.MINIMIZE] - ) # type: ignore - else: - storage.create_new_study("foo") + create_new_study(storage, "foo", [StudyDirection.MINIMIZE]) self.assertEqual(len(get_all_study_summaries(storage)), 1) app = create_app(storage) @@ -163,16 +148,8 @@ class APITestCase(TestCase): def test_delete_study(self) -> None: storage = optuna.storages.InMemoryStorage() - if version.parse(optuna_ver) >= version.Version("3.1.0.dev"): - storage.create_new_study( - study_name="foo1", directions=[StudyDirection.MINIMIZE] - ) # type: ignore - storage.create_new_study( - study_name="foo2", directions=[StudyDirection.MINIMIZE] - ) # type: ignore - else: - storage.create_new_study(study_name="foo1") - storage.create_new_study(study_name="foo2") + create_new_study(storage, "foo1", [StudyDirection.MINIMIZE]) + create_new_study(storage, "foo2", [StudyDirection.MINIMIZE]) self.assertEqual(len(get_all_study_summaries(storage)), 2) app = create_app(storage) From 0b0970088975e7c5ce904f36142770a6b73859f1 Mon Sep 17 00:00:00 2001 From: c-bata Date: Thu, 22 Dec 2022 23:12:02 +0900 Subject: [PATCH 7/7] Fix broken validations --- optuna_dashboard/_app.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index 461da83d..94009dae 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -165,9 +165,9 @@ def create_new_study( if version.parse(optuna_ver) >= version.Version("3.1.0.dev") and version.parse( optuna_ver ) != version.Version("3.1.0b0"): - study_id = storage.create_new_study(study_name, directions=directions) # type: ignore + study_id = storage.create_new_study(directions, study_name=study_name) # type: ignore else: - study_id = storage.create_new_study(study_name) + study_id = storage.create_new_study(study_name) # type: ignore storage.set_study_directions(study_id, directions) # type: ignore return study_id @@ -254,18 +254,19 @@ def create_app(storage: BaseStorage, debug: bool = False) -> Bottle: @json_api_view def create_study() -> BottleViewReturn: study_name = request.json.get("study_name", None) - directions = [ - StudyDirection.MAXIMIZE if d.lower() == "maximize" else StudyDirection.MINIMIZE - for d in request.json.get("directions", []) - ] + request_directions = [d.lower() for d in request.json.get("directions", [])] if ( study_name is None - or len(directions) == 0 - or not all([d in ("minimize", "maximize") for d in directions]) + or len(request_directions) == 0 + or not all([d in ("minimize", "maximize") for d in request_directions]) ): response.status = 400 # Bad request return {"reason": "You need to set study_name and direction"} + directions = [ + StudyDirection.MAXIMIZE if d == "maximize" else StudyDirection.MINIMIZE + for d in request_directions + ] try: study_id = create_new_study(storage, study_name, directions) except DuplicatedStudyError: