Merge pull request #258 from openml/fix_flow_tags

improve flow handling

Author: mfeurer

openml/flows/flow.py

@@ -4,6 +4,7 @@
 import xmltodict
 
 from .._api_calls import _perform_api_call
+from ..utils import extract_xml_tags
 
 
 class OpenMLFlow(object):
@@ -278,10 +279,8 @@ def _from_dict(cls, xml_dict):
         if 'oml:parameter' in dic:
             # In case of a single parameter, xmltodict returns a dictionary,
             # otherwise a list.
-            if isinstance(dic['oml:parameter'], dict):
-                oml_parameters = [dic['oml:parameter']]
-            else:
-                oml_parameters = dic['oml:parameter']
+            oml_parameters = extract_xml_tags('oml:parameter', dic,
+                                              allow_none=False)
 
             for oml_parameter in oml_parameters:
                 parameter_name = oml_parameter['oml:name']
@@ -299,27 +298,14 @@ def _from_dict(cls, xml_dict):
         if 'oml:component' in dic:
             # In case of a single component xmltodict returns a dict,
             # otherwise a list.
-            if isinstance(dic['oml:component'], dict):
-                oml_components = [dic['oml:component']]
-            else:
-                oml_components = dic['oml:component']
+            oml_components = extract_xml_tags('oml:component', dic,
+                                              allow_none=False)
 
             for component in oml_components:
                 flow = OpenMLFlow._from_dict(component)
                 components[component['oml:identifier']] = flow
         arguments['components'] = components
-
-        tags = []
-        if 'oml:tag' in dic and dic['oml:tag'] is not None:
-            # In case of a single tag xmltodict returns a dict, otherwise a list
-            if isinstance(dic['oml:tag'], dict):
-                oml_tags = [dic['oml:tag']]
-            else:
-                oml_tags = dic['oml:tag']
-
-            for tag in oml_tags:
-                tags.append(tag)
-        arguments['tags'] = tags
+        arguments['tags'] = extract_xml_tags('oml:tag', dic)
 
         arguments['model'] = None
         flow = cls(**arguments)
@@ -385,3 +371,4 @@ def _add_if_nonempty(dic, key, value):
     if value is not None:
         dic[key] = value
 
+

openml/flows/functions.py

@@ -175,7 +175,10 @@ def assert_flows_equal(flow1, flow2, ignore_parameters_on_older_children=None,
 
     # TODO as they are actually now saved during publish, it might be good to
     # check for the equality of these as well.
-    generated_by_the_server = ['flow_id', 'uploader', 'version', 'upload_date']
+    generated_by_the_server = ['flow_id', 'uploader', 'version', 'upload_date',
+                               # Tags aren't directly created by the server,
+                               # but the uploader has no control over them!
+                               'tags']
     ignored_by_python_API = ['binary_url', 'binary_format', 'binary_md5',
                              'model']
 

openml/flows/sklearn_converter.py

@@ -208,7 +208,15 @@ def _serialize_model(model):
                       parameters=parameters,
                       parameters_meta_info=parameters_meta_info,
                       external_version=external_version,
-                      tags=[],
+                      tags=['openml-python', 'sklearn', 'scikit-learn',
+                            'python',
+                            _format_external_version('sklearn',
+                                                     sklearn.__version__).replace('==', '_'),
+                            # TODO: add more tags based on the scikit-learn
+                            # module a flow is in? For example automatically
+                            # annotate a class of sklearn.svm.SVC() with the
+                            # tag svm?
+                            ],
                       language='English',
                       # TODO fill in dependencies!
                       dependencies=dependencies)

openml/runs/functions.py

@@ -12,6 +12,7 @@
 import xmltodict
 
 import openml
+import openml.utils
 from ..exceptions import PyOpenMLError
 from .. import config
 from ..flows import sklearn_to_flow, get_flow, flow_exists, _check_n_jobs, \
@@ -578,10 +579,13 @@ def _create_run_from_xml(xml):
             # only one result.. probably due to an upload error
             file_dict = run['oml:output_data']['oml:file']
             files[file_dict['oml:name']] = int(file_dict['oml:file_id'])
-        else:
+        elif isinstance(run['oml:output_data']['oml:file'], list):
             # multiple files, the normal case
             for file_dict in run['oml:output_data']['oml:file']:
                 files[file_dict['oml:name']] = int(file_dict['oml:file_id'])
+        else:
+            raise TypeError(type(run['oml:output_data']['oml:file']))
+
         if 'oml:evaluation' in run['oml:output_data']:
             # in normal cases there should be evaluations, but in case there
             # was an error these could be absent
@@ -614,14 +618,7 @@ def _create_run_from_xml(xml):
         raise ValueError('No prediction files for run %d in run '
                          'description XML' % run_id)
 
-    tags = None
-    if 'oml:tag' in run:
-        if isinstance(run['oml:tag'], str):
-            tags = [run['oml:tag']]
-        elif isinstance(run['oml:tag'], list):
-            tags = run['oml:tag']
-        else:
-            raise ValueError('Received not string and non list as tag item')
+    tags = openml.utils.extract_xml_tags('oml:tag', run)
 
     return OpenMLRun(run_id=run_id, uploader=uploader,
                      uploader_name=uploader_name, task_id=task_id,

openml/utils.py

@@ -0,0 +1,40 @@
+import six
+
+
+def extract_xml_tags(xml_tag_name, node, allow_none=True):
+    """Helper to extract xml tags from xmltodict.
+
+    Parameters
+    ----------
+    xml_tag_name : str
+        Name of the xml tag to extract from the node.
+
+    node : object
+        Node object returned by ``xmltodict`` from which ``xml_tag_name``
+        should be extracted.
+
+    allow_none : bool
+        If ``False``, the tag needs to exist in the node. Will raise a
+        ``ValueError`` if it does not.
+
+    Returns
+    -------
+    object
+    """
+    if xml_tag_name in node and node[xml_tag_name] is not None:
+        if isinstance(node[xml_tag_name], dict):
+            rval = [node[xml_tag_name]]
+        elif isinstance(node[xml_tag_name], six.string_types):
+            rval = [node[xml_tag_name]]
+        elif isinstance(node[xml_tag_name], list):
+            rval = node[xml_tag_name]
+        else:
+            raise ValueError('Received not string and non list as tag item')
+
+        return rval
+    else:
+        if allow_none:
+            return None
+        else:
+            raise ValueError("Could not find tag '%s' in node '%s'" %
+                             (xml_tag_name, str(node)))

requirements.txt

@@ -7,3 +7,4 @@ nose
 requests
 scikit-learn>=0.18
 nbformat
+python-dateutil

tests/test_flows/test_flow.py

@@ -27,13 +27,13 @@
 from openml.testing import TestBase
 from openml._api_calls import _perform_api_call
 import openml
+import openml.utils
 from openml.flows.sklearn_converter import _format_external_version
 import openml.exceptions
 
 
 class TestFlow(TestBase):
 
-
     def test_get_flow(self):
         # We need to use the production server here because 4024 is not the test
         # server
@@ -276,7 +276,14 @@ def test_sklearn_to_upload_to_flow(self):
             estimator=model, param_distributions=parameter_grid, cv=cv)
         rs.fit(X, y)
         flow = openml.flows.sklearn_to_flow(rs)
-        flow.tags.extend(['openml-python', 'unittest'])
+        # Tags may be sorted in any order (by the server). Just using one tag
+        # makes sure that the xml comparison does not fail because of that.
+        subflows = [flow]
+        while len(subflows) > 0:
+            f = subflows.pop()
+            f.tags = []
+            subflows.extend(list(f.components.values()))
+
         flow, sentinel = self._add_sentinel_to_flow_name(flow, None)
 
         flow.publish()
@@ -317,6 +324,16 @@ def test_sklearn_to_upload_to_flow(self):
                         % sentinel
 
         self.assertEqual(new_flow.name, fixture_name)
-        self.assertTrue('openml-python' in new_flow.tags)
-        self.assertTrue('unittest' in new_flow.tags)
         new_flow.model.fit(X, y)
+
+    def test_extract_tags(self):
+        flow_xml = "<oml:tag>study_14</oml:tag>"
+        flow_dict = xmltodict.parse(flow_xml)
+        tags = openml.utils.extract_xml_tags('oml:tag', flow_dict)
+        self.assertEqual(tags, ['study_14'])
+
+        flow_xml = "<oml:flow><oml:tag>OpenmlWeka</oml:tag>\n" \
+                   "<oml:tag>weka</oml:tag></oml:flow>"
+        flow_dict = xmltodict.parse(flow_xml)
+        tags = openml.utils.extract_xml_tags('oml:tag', flow_dict['oml:flow'])
+        self.assertEqual(tags, ['OpenmlWeka', 'weka'])

tests/test_flows/test_flow_functions.py

@@ -63,7 +63,6 @@ def test_are_flows_equal(self):
         for attribute, new_value in [('name', 'Tes'),
                                      ('description', 'Test flo'),
                                      ('external_version', '2'),
-                                     ('tags', ['abc', 'de']),
                                      ('language', 'english'),
                                      ('dependencies', 'ab'),
                                      ('class_name', 'Tes'),
@@ -83,7 +82,8 @@ def test_are_flows_equal(self):
                                      ('binary_url', 'openml.org'),
                                      ('binary_format', 'gzip'),
                                      ('binary_md5', '12345'),
-                                     ('model', [])]:
+                                     ('model', []),
+                                     ('tags', ['abc', 'de'])]:
             new_flow = copy.deepcopy(flow)
             setattr(new_flow, attribute, new_value)
             self.assertNotEqual(getattr(flow, attribute), getattr(new_flow, attribute))

tests/test_runs/test_run_functions.py

@@ -63,9 +63,13 @@ def _check_serialized_optimized_run(self, run_id):
         # downloads the best model based on the optimization trace
         # suboptimal (slow), and not guaranteed to work if evaluation
         # engine is behind. TODO: mock this? We have the arff already on the server
-        self._wait_for_processed_run(run_id, 80)
-        model_prime = openml.runs.initialize_model_from_trace(run_id, 0, 0)
-
+        self._wait_for_processed_run(run_id, 200)
+        try:
+            model_prime = openml.runs.initialize_model_from_trace(run_id, 0, 0)
+        except openml.exceptions.OpenMLServerException as e:
+            e.additional += '; run_id: ' + run_id
+            raise e
+        
         run_prime = openml.runs.run_model_on_task(task, model_prime,
                                                   avoid_duplicate_runs=False,
                                                   seed=1)
@@ -357,7 +361,7 @@ def test_get_run_trace(self):
             # in case the run did not exists yet
             run = openml.runs.run_model_on_task(task, clf, avoid_duplicate_runs=True)
             run = run.publish()
-            self._wait_for_processed_run(run.run_id, 80)
+            self._wait_for_processed_run(run.run_id, 200)
             run_id = run.run_id
         except openml.exceptions.PyOpenMLError:
             # run was already