From 135a819d798c60ee53bdcdb300810bbfae6d2f2a Mon Sep 17 00:00:00 2001 From: smohsinali Date: Fri, 13 Jan 2017 14:10:49 +0100 Subject: [PATCH 1/6] type annotate forbidden.py --- ConfigSpace/forbidden.py | 46 +++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/ConfigSpace/forbidden.py b/ConfigSpace/forbidden.py index ce7dadd2..c75fc936 100644 --- a/ConfigSpace/forbidden.py +++ b/ConfigSpace/forbidden.py @@ -32,12 +32,13 @@ import io from functools import reduce -from ConfigSpace.hyperparameters import Hyperparameter +from ConfigSpace.hyperparameters import Hyperparameter +from typing import List, Dict, Any, Union class AbstractForbiddenComponent(object): __metaclass__ = ABCMeta - + # todo : should absract methods be annotated with types? @abstractmethod def __init__(self): pass @@ -47,19 +48,19 @@ def __repr__(self): pass # http://stackoverflow.com/a/25176504/4636294 - def __eq__(self, other): + def __eq__(self, other: Any) -> bool: """Override the default Equals behavior""" if isinstance(other, self.__class__): return self.__dict__ == other.__dict__ return NotImplemented - def __ne__(self, other): + def __ne__(self, other: Any) -> bool: """Define a non-equality test""" if isinstance(other, self.__class__): return not self.__eq__(other) return NotImplemented - def __hash__(self): + def __hash__(self) -> int: """Override the default hash behavior (that returns the id or the object)""" return hash(tuple(sorted(self.__dict__.items()))) @@ -73,12 +74,13 @@ def is_forbidden(self, instantiated_hyperparameters): class AbstractForbiddenClause(AbstractForbiddenComponent): - def get_descendant_literal_clauses(self): + def get_descendant_literal_clauses(self) -> List['AbstractForbiddenClause']: return [self] class SingleValueForbiddenClause(AbstractForbiddenClause): - def __init__(self, hyperparameter, value): + # todo: possible types of hyperparameter valuees? + def __init__(self, hyperparameter: Hyperparameter, value: Any) -> None: super(SingleValueForbiddenClause, self).__init__() if not isinstance(hyperparameter, Hyperparameter): raise TypeError("'%s' is not of type %s." % @@ -90,7 +92,7 @@ def __init__(self, hyperparameter, value): "'%s'" % (hyperparameter, str(value))) self.value = value - def is_forbidden(self, instantiated_hyperparameters, strict=True): + def is_forbidden(self, instantiated_hyperparameters: Hyperparameter, strict: bool=True) -> bool: value = instantiated_hyperparameters.get(self.hyperparameter.name) if value is None: @@ -110,7 +112,8 @@ def _is_forbidden(self, target_instantiated_hyperparameter): class MultipleValueForbiddenClause(AbstractForbiddenClause): - def __init__(self, hyperparameter, values): + # todo: type of vals of hp + def __init__(self, hyperparameter: Hyperparameter, values: Any) -> None: super(MultipleValueForbiddenClause, self).__init__() if not isinstance(hyperparameter, Hyperparameter): raise TypeError("Argument 'hyperparameter' is not of type %s." % @@ -123,7 +126,7 @@ def __init__(self, hyperparameter, values): "'%s'" % (hyperparameter, str(value))) self.values = values - def is_forbidden(self, instantiated_hyperparameters, strict=True): + def is_forbidden(self, instantiated_hyperparameters: Hyperparameter, strict: bool=True) -> bool: value = instantiated_hyperparameters.get(self.hyperparameter.name) if value is None: @@ -143,31 +146,31 @@ def _is_forbidden(self, target_instantiated_hyperparameter): class ForbiddenEqualsClause(SingleValueForbiddenClause): - def __repr__(self): + def __repr__(self) -> str: return "Forbidden: %s == %s" % (self.hyperparameter.name, repr(self.value)) - def _is_forbidden(self, value): + def _is_forbidden(self, value: Any) -> bool: return value == self.value class ForbiddenInClause(MultipleValueForbiddenClause): - def __init__(self, hyperparameter, values): + def __init__(self, hyperparameter: Hyperparameter, values: Any) -> None: super(ForbiddenInClause, self).__init__(hyperparameter, values) self.values = set(self.values) - def __repr__(self): + def __repr__(self) -> str: return "Forbidden: %s in %s" % ( self.hyperparameter.name, "{" + ", ".join((repr(value) for value in sorted(self.values))) + "}") - def _is_forbidden(self, value): + def _is_forbidden(self, value: Any) -> bool: return value in self.values class AbstractForbiddenConjunction(AbstractForbiddenComponent): - def __init__(self, *args): + def __init__(self, *args: AbstractForbiddenComponent) -> None: super(AbstractForbiddenConjunction, self).__init__() # Test the classes for idx, component in enumerate(args): @@ -183,7 +186,7 @@ def __init__(self, *args): def __repr__(self): pass - def get_descendant_literal_clauses(self): + def get_descendant_literal_clauses(self) -> List[Hyperparameter]: children = [] for component in self.components: if isinstance(component, AbstractForbiddenConjunction): @@ -192,7 +195,7 @@ def get_descendant_literal_clauses(self): children.append(component) return children - def is_forbidden(self, instantiated_hyperparameters, strict=True): + def is_forbidden(self, instantiated_hyperparameters: Hyperparameter, strict: bool=True) -> bool: ihp_names = list(instantiated_hyperparameters.keys()) dlcs = self.get_descendant_literal_clauses() @@ -211,8 +214,7 @@ def is_forbidden(self, instantiated_hyperparameters, strict=True): # outcomes evaluations = [] for component in self.components: - e = component.is_forbidden(instantiated_hyperparameters, - strict=strict) + e = component.is_forbidden(instantiated_hyperparameters) evaluations.append(e) return self._is_forbidden(evaluations) @@ -222,7 +224,7 @@ def _is_forbidden(self, evaluations): class ForbiddenAndConjunction(AbstractForbiddenConjunction): - def __repr__(self): + def __repr__(self) -> str: retval = io.StringIO() retval.write("(") for idx, component in enumerate(self.components): @@ -232,5 +234,5 @@ def __repr__(self): retval.write(")") return retval.getvalue() - def _is_forbidden(self, evaluations): + def _is_forbidden(self, evaluations: List[bool]) -> bool: return reduce(operator.and_, evaluations) From 3e29549a263860c4339670d57002294c35809c43 Mon Sep 17 00:00:00 2001 From: smohsinali Date: Sun, 15 Jan 2017 16:05:35 +0100 Subject: [PATCH 2/6] fix --- ConfigSpace/hyperparameters.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ConfigSpace/hyperparameters.py b/ConfigSpace/hyperparameters.py index 4c049fa6..87e231d7 100644 --- a/ConfigSpace/hyperparameters.py +++ b/ConfigSpace/hyperparameters.py @@ -103,12 +103,12 @@ def get_num_neighbors(self): class Constant(Hyperparameter): def __init__(self, name, value): super(Constant, self).__init__(name) - allowed_types = [] - allowed_types.extend(int) - allowed_types.append(float) - allowed_types.extend(str) - allowed_types.append(str) - allowed_types = tuple(allowed_types) + allowed_types_list = [] # type: List[type] + allowed_types_list.append(int) + allowed_types_list.append(float) + allowed_types_list.append(str) + + allowed_types = tuple(allowed_types_list) if not isinstance(value, allowed_types) or \ isinstance(value, bool): From dec99c32e5fea19db792be7b93fdd00a03f9a72c Mon Sep 17 00:00:00 2001 From: smohsinali Date: Sun, 15 Jan 2017 17:15:12 +0100 Subject: [PATCH 3/6] io.reduce->reduce --- ConfigSpace/conditions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ConfigSpace/conditions.py b/ConfigSpace/conditions.py index 7a7b9860..7c592aeb 100644 --- a/ConfigSpace/conditions.py +++ b/ConfigSpace/conditions.py @@ -302,7 +302,7 @@ def __repr__(self): return retval.getvalue() def _evaluate(self, evaluations): - return io.reduce(operator.and_, evaluations) + return reduce(operator.and_, evaluations) class OrConjunction(AbstractConjunction): From ddc7033287dc0d117305423aba16bb9242ed3272 Mon Sep 17 00:00:00 2001 From: smohsinali Date: Fri, 20 Jan 2017 01:07:13 +0100 Subject: [PATCH 4/6] made changes according to comments on pull request --- ConfigSpace/forbidden.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/ConfigSpace/forbidden.py b/ConfigSpace/forbidden.py index c75fc936..8b8bf81c 100644 --- a/ConfigSpace/forbidden.py +++ b/ConfigSpace/forbidden.py @@ -38,7 +38,6 @@ class AbstractForbiddenComponent(object): __metaclass__ = ABCMeta - # todo : should absract methods be annotated with types? @abstractmethod def __init__(self): pass @@ -74,12 +73,11 @@ def is_forbidden(self, instantiated_hyperparameters): class AbstractForbiddenClause(AbstractForbiddenComponent): - def get_descendant_literal_clauses(self) -> List['AbstractForbiddenClause']: + def get_descendant_literal_clauses(self) -> List[AbstractForbiddenComponent]: return [self] class SingleValueForbiddenClause(AbstractForbiddenClause): - # todo: possible types of hyperparameter valuees? def __init__(self, hyperparameter: Hyperparameter, value: Any) -> None: super(SingleValueForbiddenClause, self).__init__() if not isinstance(hyperparameter, Hyperparameter): @@ -92,7 +90,7 @@ def __init__(self, hyperparameter: Hyperparameter, value: Any) -> None: "'%s'" % (hyperparameter, str(value))) self.value = value - def is_forbidden(self, instantiated_hyperparameters: Hyperparameter, strict: bool=True) -> bool: + def is_forbidden(self, instantiated_hyperparameters: Dict[str, Union[None, str, float, int]], strict: bool=True) -> bool: value = instantiated_hyperparameters.get(self.hyperparameter.name) if value is None: @@ -112,7 +110,6 @@ def _is_forbidden(self, target_instantiated_hyperparameter): class MultipleValueForbiddenClause(AbstractForbiddenClause): - # todo: type of vals of hp def __init__(self, hyperparameter: Hyperparameter, values: Any) -> None: super(MultipleValueForbiddenClause, self).__init__() if not isinstance(hyperparameter, Hyperparameter): @@ -126,7 +123,7 @@ def __init__(self, hyperparameter: Hyperparameter, values: Any) -> None: "'%s'" % (hyperparameter, str(value))) self.values = values - def is_forbidden(self, instantiated_hyperparameters: Hyperparameter, strict: bool=True) -> bool: + def is_forbidden(self, instantiated_hyperparameters: Dict[str, Union[None, str, float, int]], strict: bool=True) -> bool: value = instantiated_hyperparameters.get(self.hyperparameter.name) if value is None: @@ -155,7 +152,7 @@ def _is_forbidden(self, value: Any) -> bool: class ForbiddenInClause(MultipleValueForbiddenClause): - def __init__(self, hyperparameter: Hyperparameter, values: Any) -> None: + def __init__(self, hyperparameter: Dict[str, Union[None, str, float, int]], values: Any) -> None: super(ForbiddenInClause, self).__init__(hyperparameter, values) self.values = set(self.values) @@ -185,7 +182,7 @@ def __init__(self, *args: AbstractForbiddenComponent) -> None: @abstractmethod def __repr__(self): pass - + # todo: recheck is return type should be AbstractForbiddenComponent or AbstractForbiddenConjunction or Hyperparameter def get_descendant_literal_clauses(self) -> List[Hyperparameter]: children = [] for component in self.components: @@ -195,7 +192,7 @@ def get_descendant_literal_clauses(self) -> List[Hyperparameter]: children.append(component) return children - def is_forbidden(self, instantiated_hyperparameters: Hyperparameter, strict: bool=True) -> bool: + def is_forbidden(self, instantiated_hyperparameters: Dict[str, Union[None, str, float, int]], strict: bool=True) -> bool: ihp_names = list(instantiated_hyperparameters.keys()) dlcs = self.get_descendant_literal_clauses() From 18ea83fc218a90ae36f1a2a6246db2a3f1326474 Mon Sep 17 00:00:00 2001 From: smohsinali Date: Fri, 20 Jan 2017 12:00:30 +0100 Subject: [PATCH 5/6] Merge branch 'master' of https://github.com/automl/ConfigSpace into typecheck_hyperparameter_p3 # Conflicts: # ConfigSpace/hyperparameters.py --- ConfigSpace/forbidden.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ConfigSpace/forbidden.py b/ConfigSpace/forbidden.py index 8b8bf81c..c226689c 100644 --- a/ConfigSpace/forbidden.py +++ b/ConfigSpace/forbidden.py @@ -211,7 +211,8 @@ def is_forbidden(self, instantiated_hyperparameters: Dict[str, Union[None, str, # outcomes evaluations = [] for component in self.components: - e = component.is_forbidden(instantiated_hyperparameters) + e = component.is_forbidden(instantiated_hyperparameters, + strict=strict) evaluations.append(e) return self._is_forbidden(evaluations) From 2a4c1db4d963d637cdf995d41491889d8acaf2b1 Mon Sep 17 00:00:00 2001 From: smohsinali Date: Wed, 25 Jan 2017 04:55:02 +0100 Subject: [PATCH 6/6] add hyperparameter attribute to abstractforbiddencomponent --- ConfigSpace/forbidden.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/ConfigSpace/forbidden.py b/ConfigSpace/forbidden.py index c226689c..0fab0aa2 100644 --- a/ConfigSpace/forbidden.py +++ b/ConfigSpace/forbidden.py @@ -38,6 +38,7 @@ class AbstractForbiddenComponent(object): __metaclass__ = ABCMeta + hyperparameter = None # type: Hyperparameter @abstractmethod def __init__(self): pass @@ -68,7 +69,7 @@ def get_descendant_literal_clauses(self): pass @abstractmethod - def is_forbidden(self, instantiated_hyperparameters): + def is_forbidden(self, instantiated_hyperparameters, strict): pass @@ -182,8 +183,9 @@ def __init__(self, *args: AbstractForbiddenComponent) -> None: @abstractmethod def __repr__(self): pass - # todo: recheck is return type should be AbstractForbiddenComponent or AbstractForbiddenConjunction or Hyperparameter - def get_descendant_literal_clauses(self) -> List[Hyperparameter]: + + # todo:recheck is return type should be AbstractForbiddenComponent or AbstractForbiddenConjunction or Hyperparameter + def get_descendant_literal_clauses(self) -> List[AbstractForbiddenComponent]: children = [] for component in self.components: if isinstance(component, AbstractForbiddenConjunction):