Repository: optuna/optuna
Pull request: Applied `@abc.abstractmethod` decorator to the abstract methods of `BaseTrial` and fixed `ChainerMNTrial`.
issue #1084

This PR applied `@abc.abstractmethod` decorator to the abstract methods of `BaseTrial`


Problem statement:
Apply `@abc.abstractmethod` decorator to the abstract methods of `BaseTrial`.
Currently, the abstract methods of the `BaseTrial` class aren't decorated by `@abc.abstractmethod`, but I think that it is not intentional. We should apply the decorator to the methods since it is a good Python convention to follow.

Hints from issue discussion:

Commit message: The function suggest_float was not implemented in the class 'ChainerMNTrial'.
So, I implemented it.

Patch to review:
diff --git a/optuna/integration/chainermn.py b/optuna/integration/chainermn.py
index 3b6203506c5..772a5e0b7a3 100644
--- a/optuna/integration/chainermn.py
+++ b/optuna/integration/chainermn.py
@@ -189,6 +189,17 @@ def __init__(self, trial, comm):
         self.delegate = trial
         self.comm = comm
 
+    def suggest_float(self, name, low, high, *, log=False):
+        # type: (str, float, float, bool) -> float
+
+        def func():
+            # type: () -> float
+
+            assert self.delegate is not None
+            return self.delegate.suggest_float(name, low, high)
+
+        return self._call_with_mpi(func)
+
     def suggest_uniform(self, name, low, high):
         # type: (str, float, float) -> float
 
diff --git a/optuna/trial.py b/optuna/trial.py
index 3edf5e9f93a..ef691b5a888 100644
--- a/optuna/trial.py
+++ b/optuna/trial.py
@@ -29,81 +29,96 @@ class BaseTrial(object, metaclass=abc.ABCMeta):
     Note that this class is not supposed to be directly accessed by library users.
     """
 
+    @abc.abstractmethod
     def suggest_float(self, name, low, high, *, log=False):
         # type: (str, float, float, bool) -> float
 
         raise NotImplementedError
 
+    @abc.abstractmethod
     def suggest_uniform(self, name, low, high):
         # type: (str, float, float) -> float
 
         raise NotImplementedError
 
+    @abc.abstractmethod
     def suggest_loguniform(self, name, low, high):
         # type: (str, float, float) -> float
 
         raise NotImplementedError
 
+    @abc.abstractmethod
     def suggest_discrete_uniform(self, name, low, high, q):
         # type: (str, float, float, float) -> float
 
         raise NotImplementedError
 
+    @abc.abstractmethod
     def suggest_int(self, name, low, high, step=1):
         # type: (str, int, int, int) -> int
 
         raise NotImplementedError
 
+    @abc.abstractmethod
     def suggest_categorical(self, name, choices):
         # type: (str, Sequence[CategoricalChoiceType]) -> CategoricalChoiceType
 
         raise NotImplementedError
 
+    @abc.abstractmethod
     def report(self, value, step):
         # type: (float, int) -> None
 
         raise NotImplementedError
 
+    @abc.abstractmethod
     def should_prune(self, step=None):
         # type: (Optional[int]) -> bool
 
         raise NotImplementedError
 
+    @abc.abstractmethod
     def set_user_attr(self, key, value):
         # type: (str, Any) -> None
 
         raise NotImplementedError
 
+    @abc.abstractmethod
     def set_system_attr(self, key, value):
         # type: (str, Any) -> None
 
         raise NotImplementedError
 
     @property
+    @abc.abstractmethod
     def params(self):
         # type: () -> Dict[str, Any]
 
         raise NotImplementedError
 
     @property
+    @abc.abstractmethod
     def distributions(self):
         # type: () -> Dict[str, BaseDistribution]
 
         raise NotImplementedError
 
     @property
+    @abc.abstractmethod
     def user_attrs(self):
         # type: () -> Dict[str, Any]
 
         raise NotImplementedError
 
     @property
+    @abc.abstractmethod
     def system_attrs(self):
         # type: () -> Dict[str, Any]
 
         raise NotImplementedError
 
     @property
+    @abc.abstractmethod
     def datetime_start(self):
         # type: () -> Optional[datetime]
 


Code review comments:
diff_hunk: @@ -189,6 +189,17 @@ def __init__(self, trial, comm):
         self.delegate = trial
         self.comm = comm
 
+    def suggest_float(self, name, low, high, *, log=False):
+        # type: (str, float, float, bool) -> float
+
+        def func():
+            # type: () -> float
+
+            assert self.delegate is not None
+            return self.delegate.suggest_float(name, low, high)
path: optuna/integration/chainermn.py
text: @user1:
It seems like the keyword argument of `log` is ignored.

Isn't it better to pass `log` to `self.delegate.suggest_float`?

@author:
Thank you!
I will fix it.