Typecheck conditions p3 #17

smohsinali · 2017-01-13T13:33:50Z

No description provided.

…ypecheck_conditions_p3 # Conflicts: # ConfigSpace/conditions.py # ConfigSpace/hyperparameters.py

smohsinali · 2017-01-20T00:18:17Z

ERROR: test_greater_and_less_condition (test.test_conditions.TestConditions)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/smohsinali/ConfigSpace/test/test_conditions.py", line 131, in test_greater_and_less_condition
    self.assertFalse(gt.evaluate({hp.name: None}))
  File "/home/travis/build/smohsinali/ConfigSpace/ConfigSpace/conditions.py", line 114, in evaluate
    return self._evaluate(instantiated_parent_hyperparameter[hp_name])
  File "/home/travis/build/smohsinali/ConfigSpace/ConfigSpace/conditions.py", line 253, in _evaluate
    return value > self.value
TypeError: unorderable types: NoneType() > int()

coveralls · 2017-01-27T06:02:28Z

Coverage increased (+0.05%) to 67.39% when pulling a2279ff on smohsinali:typecheck_conditions_p3 into a7f2c84 on automl:master.

…sinali/ConfigSpace into typecheck_conditions_p3

coveralls · 2017-02-03T01:59:18Z

Coverage increased (+0.09%) to 67.414% when pulling 3b3d4f5 on smohsinali:typecheck_conditions_p3 into 4aa95af on automl:master.

mfeurer · 2017-02-03T06:29:32Z

Is this PR ready for a review?

smohsinali · 2017-02-03T06:38:13Z

yes its ready:)

mfeurer · 2017-02-03T07:20:14Z

ConfigSpace/conditions.py

@@ -64,19 +65,19 @@ def evaluate(self, instantiated_parent_hyperparameter):
        pass



Please annotate all abstract methods as well.

mfeurer · 2017-02-03T07:22:41Z

ConfigSpace/conditions.py

@@ -134,27 +136,28 @@ def __init__(self, *args):
                raise ValueError("All Conjunctions and Conditions must have "
                                 "the same child.")

-    def get_descendant_literal_conditions(self):
-        children = []
+    def get_descendant_literal_conditions(self) -> List[ConditionComponent]:


Wouldn't this only return AbstractCondition? If it is a conjunction, it is replaced by all conditions which make up the conjunction.

first line of this function initializes a list which is filled and returned. that list might only have one item but it will make the return type list

mfeurer · 2017-02-03T07:23:00Z

ConfigSpace/conditions.py

-    def get_descendant_literal_conditions(self):
-        children = []
+    def get_descendant_literal_conditions(self) -> List[ConditionComponent]:
+        children = []  # type: List[ConditionComponent]


AbstractCondition as well?

same as above

mfeurer · 2017-02-03T07:23:30Z

ConfigSpace/conditions.py

        for component in self.components:
            parents.extend(component.get_parents())
        return parents

+    # TODO: find typeof _evaluate?


mfeurer · 2017-02-03T07:24:04Z

ConfigSpace/conditions.py

@@ -180,7 +183,8 @@ def _evaluate(self, evaluations):


 class EqualsCondition(AbstractCondition):
-    def __init__(self, child, parent, value):
+    # ToDo: typeof value?


Since all hyperparameter values so far are Union[str, float, int], this can be the same.

mfeurer · 2017-02-03T07:26:50Z

ConfigSpace/conditions.py

@@ -301,18 +309,19 @@ def __repr__(self):
        retval.write(")")
        return retval.getvalue()

-    def _evaluate(self, evaluations):
+    # TODO: type of evaluations?


Please remove this todo.

mfeurer · 2017-02-03T07:27:54Z

ConfigSpace/hyperparameters.py

        return 0

-    def get_neighbors(self, value, rs, number, transform=False):
+    def get_neighbors(self, value: Any, rs: Any, number: int, transform: bool = False) -> List:


rs should not be of type Any, but np.random.RandomState

mfeurer · 2017-02-03T07:28:16Z

ConfigSpace/hyperparameters.py

        return True

-    def get_num_neighbors(self):
+    def get_num_neighbors(self, value=None) -> np.inf:


np.inf is of type np.float, right?

mfeurer · 2017-02-03T07:28:53Z

ConfigSpace/hyperparameters.py

+        return True
+
+    def get_order(self, value: Union[int, float]) -> Union[int, float]:
+        return value


What is this supposed to do?

mfeurer · 2017-02-03T07:30:44Z

ConfigSpace/hyperparameters.py

+
+    def get_order(self, value: Union[None, int, str, float]) -> Union[None, int, str, float]:
+        """
+        returns the seuence position/order of a certain value from the sequence


This is not what the function does.

mfeurer · 2017-02-03T07:41:21Z

This review submission did not work as expected. Please fix the mentioned issues. We should then discuss the how the approach of condition can be generalized in person.

coveralls · 2017-02-09T10:34:02Z

Coverage increased (+0.7%) to 68.05% when pulling a472ba0 on smohsinali:typecheck_conditions_p3 into 4aa95af on automl:master.

coveralls · 2017-02-09T10:52:43Z

Coverage increased (+0.8%) to 68.08% when pulling b75bf78 on smohsinali:typecheck_conditions_p3 into 4aa95af on automl:master.

coveralls · 2017-02-09T11:07:42Z

Coverage increased (+0.8%) to 68.08% when pulling b75bf78 on smohsinali:typecheck_conditions_p3 into 4aa95af on automl:master.

mfeurer · 2017-02-10T16:59:06Z

@smohsinali this PR has a conflict now. Please resolve.

coveralls · 2017-02-23T01:14:14Z

Coverage increased (+1.0%) to 68.861% when pulling b46ae45 on smohsinali:typecheck_conditions_p3 into be0ef51 on automl:master.

smohsinali added 3 commits January 13, 2017 14:32

type annotate conditions.py

694835a

fix

1fe9222

Merge branch 'master' of https://github.com/automl/ConfigSpace into t…

9888836

…ypecheck_conditions_p3 # Conflicts: # ConfigSpace/conditions.py # ConfigSpace/hyperparameters.py

smohsinali added 2 commits January 25, 2017 10:54

is_legal

32cdb10

add support for ordinal in greaterthan and lessthan comparisons

a2279ff

smohsinali added 2 commits February 3, 2017 01:48

Merge branch 'typecheck_hyperparameter_p3' of https://github.com/smoh…

30a48ae

…sinali/ConfigSpace into typecheck_conditions_p3

make inequality checks more general

3b3d4f5

mfeurer requested changes Feb 3, 2017

View reviewed changes

smohsinali added 8 commits February 3, 2017 14:07

fix

3303c02

fix

0f33105

fix hp

043c687

conditioncomponent->abstractcondition

5560274

conditioncomponent->abstractcondition

fa51b6d

add compare function

1e15ef6

add compare function

a472ba0

add compare function

b75bf78

Merge branch 'master' into typecheck_conditions_p3

b46ae45

mfeurer approved these changes Feb 23, 2017

View reviewed changes

mfeurer merged commit 9cea3b4 into automl:master Feb 23, 2017

smohsinali deleted the typecheck_conditions_p3 branch June 8, 2017 02:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Typecheck conditions p3 #17

Typecheck conditions p3 #17

smohsinali commented Jan 13, 2017

smohsinali commented Jan 20, 2017 •

edited

coveralls commented Jan 27, 2017 •

edited

coveralls commented Feb 3, 2017 •

edited

mfeurer commented Feb 3, 2017

smohsinali commented Feb 3, 2017

mfeurer Feb 3, 2017

smohsinali Feb 3, 2017

mfeurer Feb 3, 2017

smohsinali Feb 3, 2017

mfeurer Feb 3, 2017

smohsinali Feb 3, 2017

mfeurer Feb 3, 2017

smohsinali Feb 3, 2017

mfeurer Feb 3, 2017

smohsinali Feb 3, 2017

mfeurer Feb 3, 2017

smohsinali Feb 3, 2017

mfeurer Feb 3, 2017

smohsinali Feb 3, 2017

mfeurer Feb 3, 2017

mfeurer Feb 3, 2017

mfeurer Feb 3, 2017

mfeurer commented Feb 3, 2017

coveralls commented Feb 9, 2017 •

edited

coveralls commented Feb 9, 2017 •

edited

coveralls commented Feb 9, 2017 •

edited

mfeurer commented Feb 10, 2017

coveralls commented Feb 23, 2017 •

edited

		@@ -64,19 +65,19 @@ def evaluate(self, instantiated_parent_hyperparameter):
		pass

Typecheck conditions p3 #17

Typecheck conditions p3 #17

Conversation

smohsinali commented Jan 13, 2017

smohsinali commented Jan 20, 2017 • edited

coveralls commented Jan 27, 2017 • edited

coveralls commented Feb 3, 2017 • edited

mfeurer commented Feb 3, 2017

smohsinali commented Feb 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mfeurer commented Feb 3, 2017

coveralls commented Feb 9, 2017 • edited

coveralls commented Feb 9, 2017 • edited

coveralls commented Feb 9, 2017 • edited

mfeurer commented Feb 10, 2017

coveralls commented Feb 23, 2017 • edited

smohsinali commented Jan 20, 2017 •

edited

coveralls commented Jan 27, 2017 •

edited

coveralls commented Feb 3, 2017 •

edited

coveralls commented Feb 9, 2017 •

edited

coveralls commented Feb 9, 2017 •

edited

coveralls commented Feb 9, 2017 •

edited

coveralls commented Feb 23, 2017 •

edited