Repository: optuna/optuna
Pull request: Add delete_study api for storage package
close https://github.com/pfnet/optuna/issues/421

* [x] Add storage api to delete study
* [x] Add `delete-study` subcommand on CLI

Problem statement:
No way to delete study
Searching the documentation and API shows no way to delete a study, e.g. clear all results and start fresh.  Is this possible, say through `optuna.delete_study(study_name='...', storage='mysql...')`.

In the case of a local db file, it an easily be deleted, however using a remote mysql server seems to require deleting all records from the database.

Hints from issue discussion:
Sorry, Optuna does not currently provide study deletion API.
We want to implement it, but the detailed schedule has not been decided.
After #422 has been merged, adding the API seems easy (thanks to @c-bata).
Of course, if you (or others) are interested in implementing the API, contributions are very welcome!
Thank-you for the reply.  I will look into it if I get a chance, but for now I suppose I will get by with incremental study names.
Hi! I added the ORM-level ON DELETE cascade option at #422. If we just deletes study object by using SQLAlchemy, then all associated records (like trials, study_user_attributes, ...) will be deleted.

The delete_study method of storage module is like following.
https://github.com/c-bata/optuna/compare/733a66c...abd0492?expand=1

The user-level API design needs more consideration. It might be enough if Study class provides `delete` method. And perhaps, it might be useful if CLI provides the `delete-study` subcommand.

Commit message: Add tests for delete-study subcommand

Patch to review:
diff --git a/optuna/cli.py b/optuna/cli.py
index cef3cd2230b..a0e6d1134e5 100644
--- a/optuna/cli.py
+++ b/optuna/cli.py
@@ -82,6 +82,27 @@ def take_action(self, parsed_args):
         print(study_name)
 
 
+class DeleteStudy(BaseCommand):
+    def get_parser(self, prog_name):
+        # type: (str) -> ArgumentParser
+
+        parser = super(DeleteStudy, self).get_parser(prog_name)
+        parser.add_argument(
+            '--study-name',
+            default=None,
+            help='A human-readable name of a study to distinguish it from others.')
+        return parser
+
+    def take_action(self, parsed_args):
+        # type: (Namespace) -> None
+
+        config = optuna.config.load_optuna_config(self.app_args.config)
+        storage_url = get_storage_url(self.app_args.storage, config)
+        storage = optuna.storages.RDBStorage(storage_url)
+        study_id = storage.get_study_id_from_name(parsed_args.study_name)
+        storage.delete_study(study_id)
+
+
 class StudySetUserAttribute(BaseCommand):
     def get_parser(self, prog_name):
         # type: (str) -> ArgumentParser
@@ -258,6 +279,7 @@ def take_action(self, parsed_args):
 
 _COMMANDS = {
     'create-study': CreateStudy,
+    'delete-study': DeleteStudy,
     'study set-user-attr': StudySetUserAttribute,
     'studies': Studies,
     'dashboard': Dashboard,
diff --git a/optuna/storages/base.py b/optuna/storages/base.py
index 91e40f55ab6..0338d9f550b 100644
--- a/optuna/storages/base.py
+++ b/optuna/storages/base.py
@@ -33,6 +33,12 @@ def create_new_study(self, study_name=None):
 
         raise NotImplementedError
 
+    @abc.abstractmethod
+    def delete_study(self, study_id):
+        # type: (int) -> None
+
+        raise NotImplementedError
+
     @abc.abstractmethod
     def set_study_user_attr(self, study_id, key, value):
         # type: (int, str, Any) -> None
diff --git a/optuna/storages/in_memory.py b/optuna/storages/in_memory.py
index 2b0814518d7..6b1119fd270 100644
--- a/optuna/storages/in_memory.py
+++ b/optuna/storages/in_memory.py
@@ -54,6 +54,20 @@ def create_new_study(self, study_name=None):
 
         return IN_MEMORY_STORAGE_STUDY_ID  # TODO(akiba)
 
+    def delete_study(self, study_id):
+        # type: (int) -> None
+
+        self._check_study_id(study_id)
+
+        with self._lock:
+            self.trials = []
+            self.param_distribution = {}
+            self.direction = structs.StudyDirection.NOT_SET
+            self.study_user_attrs = {}
+            self.study_system_attrs = {}
+            self.study_name = DEFAULT_STUDY_NAME_PREFIX + IN_MEMORY_STORAGE_STUDY_UUID
+        return
+
     def set_study_direction(self, study_id, direction):
         # type: (int, structs.StudyDirection) -> None
 
diff --git a/optuna/storages/rdb/storage.py b/optuna/storages/rdb/storage.py
index 76f9fcf3690..15a56fe1a45 100644
--- a/optuna/storages/rdb/storage.py
+++ b/optuna/storages/rdb/storage.py
@@ -98,6 +98,15 @@ def create_new_study(self, study_name=None):
 
         return study.study_id
 
+    def delete_study(self, study_id):
+        # type: (int) -> None
+        session = self.scoped_session()
+
+        study = models.StudyModel.find_or_raise_by_id(study_id, session)
+        session.delete(study)
+
+        self._commit_with_integrity_check(session)
+
     @staticmethod
     def _create_unique_study_name(session):
         # type: (orm.Session) -> str
diff --git a/tests/storages_tests/test_storages.py b/tests/storages_tests/test_storages.py
index a9501deb572..d3e67109824 100644
--- a/tests/storages_tests/test_storages.py
+++ b/tests/storages_tests/test_storages.py
@@ -139,6 +139,22 @@ def test_create_new_study_with_name(storage_mode, cache_mode):
         assert study_name == storage.get_study_name_from_id(study_id)
 
 
+@parametrize_storage
+def test_delete_study(storage_init_func):
+    # type: (Callable[[], BaseStorage]) -> None
+
+    storage = storage_init_func()
+    study_id = storage.create_new_study_id()
+    storage.create_new_trial_id(study_id)
+    trials = storage.get_all_trials(study_id)
+    assert len(trials) == 1
+
+    storage.delete_study(study_id)
+    study_id = storage.create_new_study_id()
+    trials = storage.get_all_trials(study_id)
+    assert len(trials) == 0
+
+
 @pytest.mark.parametrize('storage_mode', STORAGE_MODES)
 @pytest.mark.parametrize('cache_mode', CACHE_MODES)
 def test_get_study_id_from_name_and_get_study_name_from_id(storage_mode, cache_mode):
diff --git a/tests/test_cli.py b/tests/test_cli.py
index ad05b7eb160..2f3cd54d9a3 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -141,6 +141,36 @@ def test_create_study_command_with_direction():
             subprocess.check_call(command)
 
 
+@pytest.mark.parametrize('options', [['storage'], ['config'], ['storage', 'config']])
+def test_delete_study_command(options):
+    # type: (List[str]) -> None
+
+    with StorageConfigSupplier(TEST_CONFIG_TEMPLATE) as (storage_url, config_path):
+        storage = RDBStorage(storage_url)
+        study_name = "delete-study-test"
+
+        # Create study.
+        command = ['optuna', 'create-study', '--storage', storage_url, '--study-name', study_name]
+        subprocess.check_call(command)
+        assert study_name in {s.study_name: s for s in storage.get_all_study_summaries()}
+
+        # Delete study.
+        command = ['optuna', 'delete-study', '--storage', storage_url, '--study-name', study_name]
+        subprocess.check_call(command)
+        assert study_name not in {s.study_name: s for s in storage.get_all_study_summaries()}
+
+
+def test_delete_study_command_without_storage_url():
+    # type: () -> None
+
+    dummy_home = tempfile.mkdtemp()
+    env = os.environ
+    env['HOME'] = dummy_home
+    with pytest.raises(subprocess.CalledProcessError) as err:
+        subprocess.check_output(['optuna', 'delete-study'], env=env)
+    shutil.rmtree(dummy_home)
+
+
 @pytest.mark.parametrize('options', [['storage'], ['config'], ['storage', 'config']])
 def test_study_set_user_attr_command(options):
     # type: (List[str]) -> None


Code review comments:
diff_hunk: @@ -54,6 +54,20 @@ def create_new_study(self, study_name=None):
 
         return IN_MEMORY_STORAGE_STUDY_ID  # TODO(akiba)
 
+    def delete_study(self, study_id):
+        # type: (int) -> None
+
+        self._check_study_id(study_id)
+
+        with self._lock:
+            self.trials = []
+            self.param_distribution = {}
+            self.direction = structs.StudyDirection.NOT_SET
+            self.study_user_attrs = {}
+            self.study_system_attrs = {}
+            self.study_name = DEFAULT_STUDY_NAME_PREFIX + IN_MEMORY_STORAGE_STUDY_UUID
+        return
path: optuna/storages/in_memory.py
text: @user1:
I think this `return` can be removed.
```suggestion
```