Remove f-strings because we still support Python 3.5 (#3121)

mlflow/store/model_registry/sqlalchemy_store.py

@@ -316,8 +316,8 @@ def compute_next_token(current_size):
         else:
             supported_ops = ''.join(['(' + op + ')' for op in
                                      SearchUtils.VALID_REGISTERED_MODEL_SEARCH_COMPARATORS])
-            sample_query = f'name {supported_ops} "<model_name>"'
-            raise MlflowException(f'Invalid filter string: {filter_string}'
+            sample_query = 'name {} "<model_name>"'.format(supported_ops)
+            raise MlflowException('Invalid filter string: {}'.format(filter_string) +
                                   'Search registered models supports filter expressions like:' +
                                   sample_query, error_code=INVALID_PARAMETER_VALUE)
         with self.ManagedSessionMaker() as session:
@@ -349,9 +349,9 @@ def _parse_search_registered_models_order_by(cls, order_by_list):
                     field = SqlRegisteredModel.last_updated_time
                 else:
                     raise MlflowException(
-                        f"Invalid order by key '{attribute_token}' specified."
-                        f"Valid keys are "
-                        f"'{SearchUtils.RECOMMENDED_ORDER_BY_KEYS_REGISTERED_MODELS}'",
+                        "Invalid order by key '{}' specified.".format(attribute_token) +
+                        "Valid keys are " +
+                        "'{}'".format(SearchUtils.RECOMMENDED_ORDER_BY_KEYS_REGISTERED_MODELS),
                         error_code=INVALID_PARAMETER_VALUE)
                 if ascending:
                     clauses.append(field.asc())

mlflow/utils/search_utils.py

@@ -340,10 +340,11 @@ def _validate_order_by_and_generate_token(cls, order_by):
         try:
             parsed = sqlparse.parse(order_by)
         except Exception:
-            raise MlflowException(f"Error on parsing order_by clause '{order_by}'",
+            raise MlflowException("Error on parsing order_by clause '{}'".format(order_by),
                                   error_code=INVALID_PARAMETER_VALUE)
         if len(parsed) != 1 or not isinstance(parsed[0], Statement):
-            raise MlflowException(f"Invalid order_by clause '{order_by}'. Could not be parsed.",
+            raise MlflowException("Invalid order_by clause '{}'. Could not be parsed."
+                                  .format(order_by),
                                   error_code=INVALID_PARAMETER_VALUE)
         statement = parsed[0]
         if len(statement.tokens) == 1 and isinstance(statement[0], Identifier):
@@ -358,7 +359,8 @@ def _validate_order_by_and_generate_token(cls, order_by):
                 and statement.tokens[-1].ttype == TokenType.Keyword.Order:
             token_value = cls.ORDER_BY_KEY_TIMESTAMP + ' ' + statement.tokens[-1].value
         else:
-            raise MlflowException(f"Invalid order_by clause '{order_by}'. Could not be parsed.",
+            raise MlflowException("Invalid order_by clause '{}'. Could not be parsed."
+                                  .format(order_by),
                                   error_code=INVALID_PARAMETER_VALUE)
         return token_value
 
@@ -368,12 +370,14 @@ def _parse_order_by_string(cls, order_by):
         is_ascending = True
         tokens = token_value.split()
         if len(tokens) > 2:
-            raise MlflowException(f"Invalid order_by clause '{order_by}'. Could not be parsed.",
+            raise MlflowException("Invalid order_by clause '{}'. Could not be parsed."
+                                  .format(order_by),
                                   error_code=INVALID_PARAMETER_VALUE)
         elif len(tokens) == 2:
             order_token = tokens[1].lower()
             if order_token not in cls.VALID_ORDER_BY_TAGS:
-                raise MlflowException(f"Invalid ordering key in order_by clause '{order_by}'.",
+                raise MlflowException("Invalid ordering key in order_by clause '{}'."
+                                      .format(order_by),
                                       error_code=INVALID_PARAMETER_VALUE)
             is_ascending = (order_token == cls.ASC_OPERATOR)
             token_value = tokens[0]
@@ -390,8 +394,10 @@ def parse_order_by_for_search_registered_models(cls, order_by):
         token_value, is_ascending = cls._parse_order_by_string(order_by)
         token_value = token_value.strip()
         if token_value not in cls.VALID_ORDER_BY_KEYS_REGISTERED_MODELS:
-            raise MlflowException(f"Invalid order by key '{token_value}' specified. Valid keys "
-                                  f"are '{cls.RECOMMENDED_ORDER_BY_KEYS_REGISTERED_MODELS}'",
+            raise MlflowException("Invalid order by key '{}' specified. Valid keys "
+                                  .format(token_value) +
+                                  "are '{}'"
+                                  .format(cls.RECOMMENDED_ORDER_BY_KEYS_REGISTERED_MODELS),
                                   error_code=INVALID_PARAMETER_VALUE)
         return token_value, is_ascending
 

tests/deployments/test_cli.py

@@ -74,6 +74,6 @@ def test_run_local():
     runner = CliRunner()
     res = runner.invoke(cli.run_local,
                         ['-f', f_flavor, '-m', f_model_uri, '-t', f_target, '--name', f_name])
-    assert f"Deployed locally at the key {f_name}" in res.stdout
-    assert f"using the model from {f_model_uri}." in res.stdout
-    assert f"It's flavor is {f_flavor} and config is {str({})}" in res.stdout
+    assert "Deployed locally at the key {}".format(f_name) in res.stdout
+    assert "using the model from {}.".format(f_model_uri) in res.stdout
+    assert "It's flavor is {} and config is {}".format(f_flavor, str({})) in res.stdout

tests/fastai/test_fastai_autolog.py

@@ -188,7 +188,7 @@ def test_fastai_autolog_early_stop_logs(fastai_random_data_run_with_callback, pa
     assert 'early_stop_mode' in params
     assert params['early_stop_mode'] == 'auto'
     assert 'early_stop_min_delta' in params
-    assert params['early_stop_min_delta'] == f'-{MIN_DELTA}'
+    assert params['early_stop_min_delta'] == '-{}'.format(MIN_DELTA)
 
     client = mlflow.tracking.MlflowClient()
     metric_history = client.get_metric_history(run.info.run_id, 'valid_loss')
@@ -212,7 +212,7 @@ def test_fastai_autolog_early_stop_no_stop_does_not_log(
     assert params['early_stop_monitor'] == 'valid_loss'
     assert 'early_stop_mode' in params
     assert 'early_stop_min_delta' in params
-    assert params['early_stop_min_delta'] == f'-{99999999}'
+    assert params['early_stop_min_delta'] == '-{}'.format(99999999)
     """
     assert 'stopped_epoch' in metrics
     assert metrics['stopped_epoch'] == 0

tests/resources/mlflow-test-plugin/mlflow_test_plugin/fake_deployment_plugin.py

@@ -29,8 +29,8 @@ def predict(self, deployment_name, df):
 
 
 def run_local(name, model_uri, flavor=None, config=None):
-    print(f"Deployed locally at the key {name} using the model from {model_uri}. "
-          f"It's flavor is {flavor} and config is {config}")
+    print("Deployed locally at the key {} using the model from {}. ".format(name, model_uri) +
+          "It's flavor is {} and config is {}".format(flavor, config))
 
 
 def target_help():

tests/store/model_registry/test_sqlalchemy_store.py

@@ -214,7 +214,7 @@ def test_list_registered_model(self):
                          set(["A", "BB", "AB", "BBC"]))
 
     def test_list_registered_model_paginated_last_page(self):
-        rms = [self._rm_maker(f"RM{i:03}").name for i in range(50)]
+        rms = [self._rm_maker("RM{:03}".format(i)).name for i in range(50)]
 
         # test flow with fixed max_results
         returned_rms = []
@@ -228,7 +228,7 @@ def test_list_registered_model_paginated_last_page(self):
         self.assertEqual(set(rms), set(returned_rms))
 
     def test_list_registered_model_paginated_returns_in_correct_order(self):
-        rms = [self._rm_maker(f"RM{i:03}").name for i in range(50)]
+        rms = [self._rm_maker("RM{:03}".format(i)).name for i in range(50)]
 
         # test that pagination will return all valid results in sorted order
         # by name ascending
@@ -250,7 +250,7 @@ def test_list_registered_model_paginated_returns_in_correct_order(self):
         self.assertEqual(result, rms[35:])
 
     def test_list_registered_model_paginated_errors(self):
-        rms = [self._rm_maker(f"RM{i:03}").name for i in range(50)]
+        rms = [self._rm_maker("RM{:03}".format(i)).name for i in range(50)]
         # test that providing a completely invalid page token throws
         with self.assertRaises(MlflowException) as exception_context:
             self._list_registered_models(page_token="evilhax", max_results=20)
@@ -263,7 +263,7 @@ def test_list_registered_model_paginated_errors(self):
         self.assertIn("Invalid value for request parameter max_results",
                       exception_context.exception.message)
         # list should not return deleted models
-        self.store.delete_registered_model(name=f"RM{0:03}")
+        self.store.delete_registered_model(name="RM{0:03}".format(0))
         self.assertEqual(set(self._list_registered_models(max_results=100)),
                          set(rms[1:]))
 
@@ -717,62 +717,62 @@ def test_search_registered_models(self):
         self.assertEqual(rms, names)
 
         # equality search using name should return exactly the 1 name
-        rms, _ = self._search_registered_models(f"name='{names[0]}'")
+        rms, _ = self._search_registered_models("name='{}'".format(names[0]))
         self.assertEqual(rms, [names[0]])
 
         # equality search using name that is not valid should return nothing
-        rms, _ = self._search_registered_models(f"name='{names[0] + 'cats'}'")
+        rms, _ = self._search_registered_models("name='{}'".format(names[0] + 'cats'))
         self.assertEqual(rms, [])
 
         # case-sensitive prefix search using LIKE should return all the RMs
-        rms, _ = self._search_registered_models(f"name LIKE '{prefix}%'")
+        rms, _ = self._search_registered_models("name LIKE '{}%'".format(prefix))
         self.assertEqual(rms, names)
 
         # case-sensitive prefix search using LIKE with surrounding % should return all the RMs
-        rms, _ = self._search_registered_models(f"name LIKE '%RM%'")
+        rms, _ = self._search_registered_models("name LIKE '%RM%'")
         self.assertEqual(rms, names)
 
         # case-sensitive prefix search using LIKE with surrounding % should return all the RMs
         # _e% matches test_for_search_ , so all RMs should match
-        rms, _ = self._search_registered_models(f"name LIKE '_e%'")
+        rms, _ = self._search_registered_models("name LIKE '_e%'")
         self.assertEqual(rms, names)
 
         # case-sensitive prefix search using LIKE should return just rm4
-        rms, _ = self._search_registered_models(f"name LIKE '{prefix + 'RM4A'}%'")
+        rms, _ = self._search_registered_models("name LIKE '{}%'".format(prefix + 'RM4A'))
         self.assertEqual(rms, [names[4]])
 
         # case-sensitive prefix search using LIKE should return no models if no match
-        rms, _ = self._search_registered_models(f"name LIKE '{prefix + 'cats'}%'")
+        rms, _ = self._search_registered_models("name LIKE '{}%'".format(prefix + 'cats'))
         self.assertEqual(rms, [])
 
         # confirm that LIKE is not case-sensitive
-        rms, _ = self._search_registered_models(f"name lIkE '%blah%'")
+        rms, _ = self._search_registered_models("name lIkE '%blah%'")
         self.assertEqual(rms, [])
 
-        rms, _ = self._search_registered_models(f"name like '{prefix + 'RM4A'}%'")
+        rms, _ = self._search_registered_models("name like '{}%'".format(prefix + 'RM4A'))
         self.assertEqual(rms, [names[4]])
 
         # case-insensitive prefix search using ILIKE should return both rm5 and rm6
-        rms, _ = self._search_registered_models(f"name ILIKE '{prefix + 'RM4A'}%'")
+        rms, _ = self._search_registered_models("name ILIKE '{}%'".format(prefix + 'RM4A'))
         self.assertEqual(rms, names[4:])
 
         # case-insensitive postfix search with ILIKE
-        rms, _ = self._search_registered_models(f"name ILIKE '%RM4a'")
+        rms, _ = self._search_registered_models("name ILIKE '%RM4a'")
         self.assertEqual(rms, names[4:])
 
         # case-insensitive prefix search using ILIKE should return both rm5 and rm6
-        rms, _ = self._search_registered_models(f"name ILIKE '{prefix + 'cats'}%'")
+        rms, _ = self._search_registered_models("name ILIKE '{}%'".format(prefix + 'cats'))
         self.assertEqual(rms, [])
 
         # confirm that ILIKE is not case-sensitive
-        rms, _ = self._search_registered_models(f"name iLike '%blah%'")
+        rms, _ = self._search_registered_models("name iLike '%blah%'")
         self.assertEqual(rms, [])
 
         # confirm that ILIKE works for empty query
-        rms, _ = self._search_registered_models(f"name iLike '%%'")
+        rms, _ = self._search_registered_models("name iLike '%%'")
         self.assertEqual(rms, names)
 
-        rms, _ = self._search_registered_models(f"name ilike '%RM4a'")
+        rms, _ = self._search_registered_models("name ilike '%RM4a'")
         self.assertEqual(rms, names[4:])
 
         # cannot search by invalid comparator types
@@ -800,18 +800,18 @@ def test_search_registered_models(self):
         self.assertEqual(self._search_registered_models(None, max_results=1000), (names[:-1], None))
 
         # equality search using name should return no names
-        self.assertEqual(self._search_registered_models(f"name='{names[-1]}'"), ([], None))
+        self.assertEqual(self._search_registered_models("name='{}'".format(names[-1])), ([], None))
 
         # case-sensitive prefix search using LIKE should return all the RMs
-        self.assertEqual(self._search_registered_models(f"name LIKE '{prefix}%'"),
+        self.assertEqual(self._search_registered_models("name LIKE '{}%'".format(prefix)),
                          (names[0:5], None))
 
         # case-insensitive prefix search using ILIKE should return both rm5 and rm6
-        self.assertEqual(self._search_registered_models(f"name ILIKE '{prefix + 'RM4A'}%'"),
+        self.assertEqual(self._search_registered_models("name ILIKE '{}%'".format(prefix + 'RM4A')),
                          ([names[4]], None))
 
     def test_search_registered_model_pagination(self):
-        rms = [self._rm_maker(f"RM{i:03}").name for i in range(50)]
+        rms = [self._rm_maker("RM{:03}".format(i)).name for i in range(50)]
 
         # test flow with fixed max_results
         returned_rms = []
@@ -859,7 +859,7 @@ def test_search_registered_model_order_by(self):
         # explicitly mock the creation_timestamps because timestamps seem to be unstable in Windows
         for i in range(50):
             with mock.patch("mlflow.store.model_registry.sqlalchemy_store.now", return_value=i):
-                rms.append(self._rm_maker(f"RM{i:03}").name)
+                rms.append(self._rm_maker("RM{:03}".format(i)).name)
 
         # test flow with fixed max_results and order_by (test stable order across pages)
         returned_rms = []

tests/test_no_f_strings.py

@@ -0,0 +1,41 @@
+# TODO: Remove this test script once we drop Python 3.5 support.
+
+import ast
+import os
+
+
+def search_python_scripts(dirs):
+    for d in dirs:
+        for root, dirs, files in os.walk(d):
+            for f in files:
+                if f.endswith('.py'):
+                    yield os.path.join(root, f)
+
+
+def read_file(path):
+    with open(path, encoding='utf-8') as f:
+        return f.read()
+
+
+def search_f_strings(node):
+    for c in ast.iter_child_nodes(node):
+        if isinstance(c, ast.JoinedStr):
+            yield (c.lineno, c.col_offset)
+        yield from search_f_strings(c)
+
+
+def test_no_f_strings():
+    dirs = ['mlflow', 'tests', 'examples']
+    f_strings_all = []
+
+    for path in search_python_scripts(dirs):
+        src = read_file(path)
+        root = ast.parse(src)
+        f_strings = list(search_f_strings(root))
+        if len(f_strings) > 0:
+            f_strings_all += [
+                '{}:{}:{}: {}'.format(path, *pos, 'Remove f-string')
+                for pos in f_strings
+            ]
+
+    assert len(f_strings_all) == 0, '\n' + '\n'.join(f_strings_all)

tests/tracking/test_model_registry.py

@@ -148,7 +148,7 @@ def _verify_pagination(rm_getter_with_token, expected_rms):
 
 @pytest.mark.parametrize("max_results", [1, 6, 100])
 def test_list_registered_model_flow_paginated(mlflow_client, backend_store_uri, max_results):
-    names = [f'CreateRMlist{i:03}' for i in range(20)]
+    names = ['CreateRMlist{:03}'.format(i) for i in range(20)]
     rms = [mlflow_client.create_registered_model(name) for name in names]
     for rm in rms:
         assert isinstance(rm, RegisteredModel)
@@ -176,7 +176,7 @@ def test_list_registered_model_flow_paginated(mlflow_client, backend_store_uri,
 ])
 def test_search_registered_model_flow_paginated(mlflow_client, backend_store_uri,
                                                 max_results, filter_string, filter_func):
-    names = [f'CreateRMsearch{i:03}' for i in range(29)]
+    names = ['CreateRMsearch{:03}'.format(i) for i in range(29)]
     rms = [mlflow_client.create_registered_model(name) for name in names]
     for rm in rms:
         assert isinstance(rm, RegisteredModel)