[GitHub] [incubator-mxnet] anko-intel commented on a diff in pull request #21136: Python string formatting

[GitBox <gi...@apache.org>]

anko-intel commented on code in PR #21136:
URL: https://github.com/apache/incubator-mxnet/pull/21136#discussion_r966753010


##########
benchmark/python/sparse/dot.py:
##########
@@ -148,11 +148,9 @@ def create_mini_path(mini_path, path, num_batches):
             last = _line_count(path) - num_batches * batch_size
             last = last if last >= 1 else 1
             start = int(rnd.uniform(1, last))
-            os.system("sed -n '%d,%dp' %r > %r"
-                      %(start, start + num_batches * batch_size, path, mini_path))
+            os.system(f"sed -n '{start},{start + num_batches * batch_size}p' {repr(path)} > {repr(mini_path)}")

Review Comment:
   . format will be more readable in this case in my opinion



##########
tests/nightly/model_backwards_compatibility_check/common.py:
##########
@@ -105,10 +105,10 @@ def get_top_level_folders_in_bucket(s3client, bucket_name):
     result = bucket.meta.client.list_objects(Bucket=bucket.name, Delimiter=backslash)
     folder_list = list()
     if 'CommonPrefixes' not in result:
-        logging.error('No trained models found in S3 bucket : %s for this file. '
-                      'Please train the models and run inference again' % bucket_name)
-        raise Exception("No trained models found in S3 bucket : %s for this file. "
-                        "Please train the models and run inference again" % bucket_name)
+        logging.error('No trained models found in S3 bucket : {} for this file. '

Review Comment:
   What is the reason to have to different approaches than in line 110?
   here {bucket_name} seems to be more readable



##########
tests/python/unittest/test_profiler.py:
##########
@@ -96,7 +96,7 @@ def test_profile_create_domain_dept():
 def test_profile_task():
     def makeParams():
         objects = tuple('foo' for _ in range(50))
-        template = ''.join('{%d}' % i for i in range(len(objects)))
+        template = ''.join(f'{{{i}}}' for i in range(len(objects)))

Review Comment:
   ```
   >>> template = ''.join(f'{{{i}}}' for i in range(3))
     File "<stdin>", line 1
       template = ''.join(f'{{{i}}}' for i in range(3))
                                   ^
   SyntaxError: invalid syntax
   
   ```



##########
tests/nightly/model_backwards_compatibility_check/common.py:
##########
@@ -121,10 +121,10 @@ def get_top_level_folders_in_bucket(s3client, bucket_name):
         folder_list.append(obj['Prefix'].strip(backslash))
 
     if len(folder_list) == 0:
-        logging.error('No trained models found in S3 bucket : %s for this file. '
-                      'Please train the models and run inference again' % bucket_name)
-        raise Exception("No trained models found in S3 bucket : %s for this file. "
-                        "Please train the models and run inference again" % bucket_name)
+        logging.error('No trained models found in S3 bucket : {} for this file. '
+                      'Please train the models and run inference again'.format(bucket_name))
+        raise Exception(f"No trained models found in S3 bucket : {bucket_name} for this file. "
+                        "Please train the models and run inference again")

Review Comment:
   Same comment as above
   ```suggestion
           logging.error(f'No trained models found in S3 bucket : {bucket_name} for this file. '
                         'Please train the models and run inference again')
           raise Exception(f"No trained models found in S3 bucket : {bucket_name} for this file. "
                           "Please train the models and run inference again")
   ```



##########
tests/python/unittest/test_profiler.py:
##########
@@ -189,7 +189,7 @@ def test_profile_tune_pause_resume():
 def test_profile_counter(do_enable_profiler=True):
     def makeParams():
         objects = tuple('foo' for _ in range(50))
-        template = ''.join('{%d}' % i for i in range(len(objects)))
+        template = ''.join(f'{{{i}}}' for i in range(len(objects)))

Review Comment:
   as above



##########
python/mxnet/gluon/contrib/estimator/estimator.py:
##########
@@ -167,9 +167,8 @@ def _check_devices(self, devices):
                                  "refer to mxnet.Device:{}".format(devices))
             for device in devices:
                 assert device in available_gpus or str(device).startswith('cpu'), \
-                    "%s is not available, please make sure " \
-                    "your device is in one of: mx.cpu(), %s" % \
-                    (device, ", ".join([str(device) for device in available_gpus]))
+                    f"{device} is not available, please make sure " \

Review Comment:
   Could you use .format( here. Now it is difficult to read what is going on



##########
tests/python/unittest/test_profiler.py:
##########
@@ -122,7 +122,7 @@ def get_log():
 def test_profile_frame():
     def makeParams():
         objects = tuple('foo' for _ in range(50))
-        template = ''.join('{%d}' % i for i in range(len(objects)))
+        template = ''.join(f'{{{i}}}' for i in range(len(objects)))

Review Comment:
   as above



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org