Typecheck configuration p3 #18

smohsinali · 2017-01-13T15:02:28Z

No description provided.

…ypecheck_configuration_p3 # Conflicts: # ConfigSpace/hyperparameters.py

coveralls · 2017-01-20T01:58:58Z

Coverage increased (+1.8%) to 69.09% when pulling 69ca427 on smohsinali:typecheck_configuration_p3 into a7f2c84 on automl:master.

mfeurer · 2017-01-20T08:43:31Z

ConfigSpace/configuration_space.py

        #  no guarantee that the parent of a condition was evaluated before
-        self._conditionsals = OrderedDict()
-        self.forbidden_clauses = []
+        self._conditionsals = OrderedDict()   # type: OrderedDict[str, str]


Line 169 suggests that this maps for str to Condition or Hyperparameter.

line 169 is
self._conditionsals[child_node] = child_node
from here it appears that key and value are equal. debugger also says the same

Hm, let's discuss this later.

make it a set

This is still open, right?

mfeurer · 2017-01-20T08:45:05Z

ConfigSpace/configuration_space.py

        return hyperparameter

-    def add_condition(self, condition):
+    def add_condition(self, condition: ConditionComponent) -> ConditionComponent:


Shouldn't this accept and return an abstract condition?

the first line of this function raises error if input is not of type conditioncomponent
if not isinstance(condition, ConditionComponent):
raise TypeError("The method add_condition must be called "
"with an instance of "
"HPOlibConfigSpace.condition.ConditionComponent.")

return type changed to abstractcondition

You're right, ConditionComponent is a parent class of AbstractCondition; accepting and returning a ConditionComponent is totally correct.

mfeurer · 2017-01-20T08:46:43Z

ConfigSpace/configuration_space.py

                    to_visit.appendleft(current)

-        by_level = defaultdict(list)
+        by_level = defaultdict(list)  # type: Dict['int', List[str]]


Why not #type: defaultdict[int, List[str]]?

mfeurer · 2017-01-20T08:47:19Z

ConfigSpace/configuration_space.py


-    def add_configuration_space(self, prefix, configuration_space,
-                                delimiter=":", parent_hyperparameter=None):
+    def add_configuration_space(self, prefix: str, configuration_space: ConfigSpace,


It should accept and return ConfigurationSpace, right?

mfeurer · 2017-01-20T08:49:09Z

ConfigSpace/configuration_space.py

+    def get_child_conditions_of(self, name: Union[str, Hyperparameter]) -> List[AbstractCondition]:
        if isinstance(name, Hyperparameter):
-            name = name.name
+            name = name.name  # type: ignore


What about # type: str?

i thought so too but mypy gives following error which doesnt make any sense
configuration_space.py:413: error: Some element of union has no attribute "name"

mfeurer · 2017-01-20T08:52:30Z

ConfigSpace/configuration_space.py

        self._num_hyperparameters = len(self.configuration_space._hyperparameters)
        self.origin = origin
-        self._keys = None
+        self._keys = None  # type: ignore


changed to Union[None, List[str]]

mfeurer · 2017-01-20T08:52:35Z

ConfigSpace/configuration_space.py

            # hyperparameters in the configuration are sorted in the same way as
            # they are sorted in the configuration space
-            self._values = dict()
+            self._values = dict()  # type: ignore


mfeurer · 2017-01-20T08:53:17Z

ConfigSpace/configuration_space.py

        return self._values[item]
-
-    def get(self, item, default=None):
+    # todo: find return type of get


I think this is correct.

mfeurer · 2017-01-20T08:53:41Z

ConfigSpace/configuration_space.py

            return default
-
-    def __contains__(self, item):
+    # todo: recheck


This should be bool I think.

return type changed to bool

mfeurer · 2017-01-20T08:54:19Z

ConfigSpace/configuration_space.py

-
-    def get_dictionary(self):
+    # todo: recheck
+    def get_dictionary(self) -> Any:


This returns a dictionary ;)

changed to Dict[str, Union[str, float, int]]

mfeurer · 2017-01-20T09:27:50Z

Could you please also make the sorting in line 648 deterministic to prevent stochastic failure of unit tests?

coveralls · 2017-01-20T12:50:36Z

Coverage increased (+0.01%) to 67.349% when pulling b234a5a on smohsinali:typecheck_configuration_p3 into a7f2c84 on automl:master.

coveralls · 2017-01-25T05:19:08Z

Coverage increased (+0.01%) to 67.349% when pulling d18d240 on smohsinali:typecheck_configuration_p3 into a7f2c84 on automl:master.

mfeurer · 2017-01-25T08:12:30Z

ConfigSpace/configuration_space.py

        return hyperparameter

-    def add_condition(self, condition: ConditionComponent) -> ConditionComponent:
+    def add_condition(self, condition: ConditionComponent) -> AbstractCondition:


This was correct before, please revert.

mfeurer · 2017-01-25T08:16:03Z

Please also remove all TODOs that you added regarding types; together with the two changes to go, this will be ready to merge.

coveralls · 2017-01-25T11:16:23Z

Coverage decreased (-0.01%) to 67.328% when pulling a40973a on smohsinali:typecheck_configuration_p3 into a7f2c84 on automl:master.

coveralls · 2017-01-25T11:16:23Z

Coverage decreased (-0.01%) to 67.328% when pulling a40973a on smohsinali:typecheck_configuration_p3 into a7f2c84 on automl:master.

smohsinali added 5 commits January 13, 2017 14:37

type annotate configuration.py

bdc6dbf

completed annotation of configuration_space.py

fd5791f

extend -> append

7905f34

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

a1e4eaf

…ypecheck_configuration_p3 # Conflicts: # ConfigSpace/hyperparameters.py

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

69ca427

…ypecheck_configuration_p3 # Conflicts: # ConfigSpace/hyperparameters.py

mfeurer requested changes Jan 20, 2017

View reviewed changes

make changes according to comments on pull request

b234a5a

smohsinali added 2 commits January 20, 2017 15:23

make changes according to comments on pull request

8facc4a

compare sorted ordered dicts

d18d240

mfeurer requested changes Jan 25, 2017

View reviewed changes

change _conditionals to List[str] from OrderedDict[str,str]

a40973a

mfeurer approved these changes Jan 25, 2017

View reviewed changes

mfeurer merged commit ea2a3cf into automl:master Jan 25, 2017

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

Typecheck configuration p3 #18

Typecheck configuration p3 #18

Uh oh!

Conversation

smohsinali commented Jan 13, 2017

Uh oh!

coveralls commented Jan 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smohsinali Jan 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smohsinali Jan 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mfeurer commented Jan 20, 2017

Uh oh!

coveralls commented Jan 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Jan 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mfeurer commented Jan 25, 2017

Uh oh!

coveralls commented Jan 25, 2017

Uh oh!

coveralls commented Jan 25, 2017

Uh oh!

Reviewers

coveralls commented Jan 20, 2017 •

edited

Loading

smohsinali Jan 20, 2017 •

edited

Loading

smohsinali Jan 20, 2017 •

edited

Loading

coveralls commented Jan 20, 2017 •

edited

Loading

coveralls commented Jan 25, 2017 •

edited

Loading