Allows empty string as parameter value (fixes mlflow#888) (mlflow#942)

* Allows empty string as parameter value (fixes mlflow#888)

* Tests for logging empty strings as param values

* Renames methods

* Fixes lint errors

mlflow/store/file_store.py

@@ -439,12 +439,13 @@ def get_metric_history(self, run_uuid, metric_key):
     def _get_param_from_file(parent_path, param_name):
         _validate_param_name(param_name)
         param_data = read_file_lines(parent_path, param_name)
-        if len(param_data) == 0:
-            raise Exception("Param '%s' is malformed. No data found." % param_name)
         if len(param_data) > 1:
             raise Exception("Unexpected data for param '%s'. Param recorded more than once"
                             % param_name)
-        return Param(param_name, str(param_data[0].strip()))
+        # The only cause for param_data's length to be zero is the param's
+        # value is an empty string
+        value = '' if len(param_data) == 0 else str(param_data[0].strip())
+        return Param(param_name, value)
 
     @staticmethod
     def _get_tag_from_file(parent_path, tag_name):

tests/store/test_file_store.py

@@ -335,6 +335,18 @@ def test_weird_param_names(self):
         assert param.key == WEIRD_PARAM_NAME
         assert param.value == "Value"
 
+    def test_log_empty_str(self):
+        PARAM_NAME = "new param"
+        fs = FileStore(self.test_root)
+        run_uuid = self.exp_data[0]["runs"][0]
+        fs.log_param(run_uuid, Param(PARAM_NAME, ""))
+        run = fs.get_run(run_uuid)
+        my_params = [p for p in run.data.params if p.key == PARAM_NAME]
+        assert len(my_params) == 1
+        param = my_params[0]
+        assert param.key == PARAM_NAME
+        assert param.value == ""
+
     def test_weird_metric_names(self):
         WEIRD_METRIC_NAME = "this is/a weird/but valid metric"
         fs = FileStore(self.test_root)

tests/store/test_sqlalchemy_store.py

@@ -485,6 +485,31 @@ def test_log_param_uniqueness(self):
             self.store.log_param(run.run_uuid, param2)
         self.assertIn("Changing param value is not allowed. Param with key=", e.exception.message)
 
+    def test_log_empty_str(self):
+        run = self._run_factory()
+
+        self.session.commit()
+
+        tkey = 'blahmetric'
+        tval = ''
+        param = entities.Param(tkey, tval)
+        param2 = entities.Param('new param', 'new key')
+        self.store.log_param(run.run_uuid, param)
+        self.store.log_param(run.run_uuid, param2)
+
+        actual = self.session.query(models.SqlParam).filter_by(key=tkey, value=tval)
+        self.assertIsNotNone(actual)
+
+        run = self.store.get_run(run.run_uuid)
+        self.assertEqual(2, len(run.data.params))
+
+        found = False
+        for m in run.data.params:
+            if m.key == tkey and m.value == tval:
+                found = True
+
+        self.assertTrue(found)
+
     def test_log_null_param(self):
         run = self._run_factory()