MAINT improve code upon Andreas' suggestions

Author: mfeurer

openml/flows/sklearn_converter.py

@@ -34,11 +34,7 @@ def sklearn_to_flow(o):
         rval = [sklearn_to_flow(element) for element in o]
         if isinstance(o, tuple):
             rval = tuple(rval)
-    elif o is None:
-        rval = None
-    elif isinstance(o, six.string_types):
-        rval = o
-    elif isinstance(o, (bool, int, float)):
+    elif isinstance(o, (bool, int, float, six.string_types)) or o is None:
         rval = o
     elif isinstance(o, dict):
         rval = OrderedDict()
@@ -124,10 +120,8 @@ def flow_to_sklearn(o, **kwargs):
         rval = [flow_to_sklearn(element, **kwargs) for element in o]
         if isinstance(o, tuple):
             rval = tuple(rval)
-    elif isinstance(o, (bool, int, float, six.string_types)):
+    elif isinstance(o, (bool, int, float, six.string_types)) or o is None:
         rval = o
-    elif o is None:
-        rval = None
     elif isinstance(o, OpenMLFlow):
         rval = _deserialize_model(o, **kwargs)
     else:
@@ -152,6 +146,84 @@ def _serialize_model(model):
     OpenMLFlow
 
     """
+
+    # Get all necessary information about the model objects itself
+    parameters, parameters_meta_info, sub_components, sub_components_explicit =\
+        _extract_information_from_model(model)
+
+    # Check that a component does not occur multiple times in a flow as this
+    # is not supported by OpenML
+    to_visit_stack = []
+    to_visit_stack.extend(sub_components.values())
+    known_sub_components = set()
+    while len(to_visit_stack) > 0:
+        visitee = to_visit_stack.pop()
+        if visitee.name in known_sub_components:
+            raise ValueError('Found a second occurence of component %s when '
+                             'trying to serialize %s.' % (visitee.name, model))
+        else:
+            known_sub_components.add(visitee.name)
+            to_visit_stack.extend(visitee.components.values())
+
+    # Create a flow name, which contains all components in brackets, for
+    # example RandomizedSearchCV(Pipeline(StandardScaler,AdaBoostClassifier(DecisionTreeClassifier)),StandardScaler,AdaBoostClassifier(DecisionTreeClassifier))
+    class_name = model.__module__ + "." + model.__class__.__name__
+
+    # will be part of the name (in brackets)
+    sub_components_names = ""
+    for key in sub_components:
+        if key in sub_components_explicit:
+            sub_components_names += "," + key + "=" + sub_components[key].name
+        else:
+            sub_components_names += "," + sub_components[key].name
+
+    if sub_components_names:
+        # slice operation on string in order to get rid of leading comma
+        name = '%s(%s)' % (class_name, sub_components_names[1:])
+    else:
+        name = class_name
+
+    # Get the external versions of all sub-components
+    model_package_name = model.__module__.split('.')[0]
+    module = importlib.import_module(model_package_name)
+    model_package_version_number = module.__version__
+    external_version = _format_external_version(model_package_name, model_package_version_number)
+
+    external_versions = set()
+    external_versions.add(external_version)
+    to_visit_stack = []
+    to_visit_stack.extend(sub_components.values())
+    while len(to_visit_stack) > 0:
+        visitee = to_visit_stack.pop()
+        for external_version in visitee.external_version.split(','):
+            external_versions.add(external_version)
+        to_visit_stack.extend(visitee.components.values())
+    external_versions = list(sorted(external_versions))
+    external_version = ','.join(external_versions)
+
+    flow = OpenMLFlow(name=name,
+                      class_name=class_name,
+                      description='Automatically created sub-component.',
+                      model=model,
+                      components=sub_components,
+                      parameters=parameters,
+                      parameters_meta_info=parameters_meta_info,
+                      external_version=external_version,
+                      tags=[],
+                      language='English',
+                      # TODO fill in dependencies!
+                      dependencies=None)
+
+    return flow
+
+
+def _extract_information_from_model(model):
+    # This function contains four "global" states and is quite long and
+    # complicated. If it gets to complicated to ensure it's correctness,
+    # it would be best to make it a class with the four "global" states being
+    # the class attributes and the if/elif/else in the for-loop calls to
+    # separate class methods
+
     # stores all entities that should become subcomponents
     sub_components = OrderedDict()
     # stores the keys of all subcomponents that should become
@@ -160,7 +232,6 @@ def _serialize_model(model):
     parameters_meta_info = OrderedDict()
 
     model_parameters = model.get_params(deep=False)
-
     for k, v in sorted(model_parameters.items(), key=lambda t: t[0]):
         rval = sklearn_to_flow(v)
 
@@ -191,7 +262,8 @@ def _serialize_model(model):
                     sub_components_explicit.add(sub_component_identifier)
                     sub_components[sub_component_identifier] = sub_component
                     component_reference = OrderedDict()
-                    component_reference['oml-python:serialized_object'] = 'component_reference'
+                    component_reference[
+                        'oml-python:serialized_object'] = 'component_reference'
                     component_reference['value'] = OrderedDict(
                         key=sub_component_identifier, step_name=identifier)
                     parameter_value.append(component_reference)
@@ -213,7 +285,8 @@ def _serialize_model(model):
             sub_components[k] = rval
             sub_components_explicit.add(k)
             component_reference = OrderedDict()
-            component_reference['oml-python:serialized_object'] = 'component_reference'
+            component_reference[
+                'oml-python:serialized_object'] = 'component_reference'
             component_reference['value'] = OrderedDict(key=k, step_name=None)
             component_reference = sklearn_to_flow(component_reference)
             parameters[k] = json.dumps(component_reference)
@@ -230,70 +303,7 @@ def _serialize_model(model):
         parameters_meta_info[k] = OrderedDict((('description', None),
                                                ('data_type', None)))
 
-    # Check that a component does not occur multiple times in a flow as this
-    # is not supported by OpenML
-    to_visit_stack = []
-    to_visit_stack.extend(sub_components.values())
-    known_sub_components = set()
-    while len(to_visit_stack) > 0:
-        visitee = to_visit_stack.pop()
-        if visitee.name in known_sub_components:
-            raise ValueError('Found a second occurence of component %s when '
-                             'trying to serialize %s.' % (visitee.name, model))
-        else:
-            known_sub_components.add(visitee.name)
-            to_visit_stack.extend(visitee.components.values())
-
-    # Create a flow name, which contains all components in brackets, for
-    # example RandomizedSearchCV(Pipeline(StandardScaler,AdaBoostClassifier(DecisionTreeClassifier)),StandardScaler,AdaBoostClassifier(DecisionTreeClassifier))
-    class_name = model.__module__ + "." + model.__class__.__name__
-
-    # will be part of the name (in brackets)
-    sub_components_names = ""
-    for key in sub_components:
-        if key in sub_components_explicit:
-            sub_components_names += "," + key + "=" + sub_components[key].name
-        else:
-            sub_components_names += "," + sub_components[key].name
-
-    if sub_components_names:
-        # slice operation on string in order to get rid of leading comma
-        name = '%s(%s)' % (class_name, sub_components_names[1:])
-    else:
-        name = class_name
-
-    # Get the external versions of all sub-components
-    model_package_name = model.__module__.split('.')[0]
-    module = importlib.import_module(model_package_name)
-    model_package_version_number = module.__version__
-    external_version = '%s==%s' % (model_package_name, model_package_version_number)
-
-    external_versions = set()
-    external_versions.add(external_version)
-    to_visit_stack = []
-    to_visit_stack.extend(sub_components.values())
-    while len(to_visit_stack) > 0:
-        visitee = to_visit_stack.pop()
-        for external_version in visitee.external_version.split(','):
-            external_versions.add(external_version)
-        to_visit_stack.extend(visitee.components.values())
-    external_versions = list(sorted(external_versions))
-    external_version = ','.join(external_versions)
-
-    flow = OpenMLFlow(name=name,
-                      class_name=class_name,
-                      description='Automatically created sub-component.',
-                      model=model,
-                      components=sub_components,
-                      parameters=parameters,
-                      parameters_meta_info=parameters_meta_info,
-                      external_version=external_version,
-                      tags=[],
-                      language='English',
-                      # TODO fill in dependencies!
-                      dependencies=None)
-
-    return flow
+    return parameters, parameters_meta_info, sub_components, sub_components_explicit
 
 
 def _deserialize_model(flow, **kwargs):
@@ -468,3 +478,7 @@ def _deserialize_cross_validator(value, **kwargs):
     for parameter in parameters:
         parameters[parameter] = flow_to_sklearn(parameters[parameter])
     return model_class(**parameters)
+
+
+def _format_external_version(model_package_name, model_package_version_number):
+    return '%s==%s' % (model_package_name, model_package_version_number)

tests/flows/test_sklearn.py

@@ -20,6 +20,7 @@
 import sklearn.tree
 
 from openml.flows import OpenMLFlow, sklearn_to_flow, flow_to_sklearn
+from openml.flows.sklearn_converter import _format_external_version
 
 
 __version__ = 0.1
@@ -480,5 +481,10 @@ def test_subflow_version_propagated(self):
         # different value, it is still correct as it is a propagation of the
         # subclasses' module name
         self.assertIn(flow.external_version,
-                      ['dummy_learn==1.0,sklearn==0.18.1',
-                       'sklearn==0.18.1,tests==0.1'])
+                      ['%s,%s' % (
+                          _format_external_version('dummy_learn', '1.0'),
+                          _format_external_version('sklearn', sklearn.__version__)),
+                       '%s,%s' % (
+                           _format_external_version('sklearn', sklearn.__version__),
+                           _format_external_version('tests', '1.0'))
+                       ])