Leverage dict comprehension in PEP-0274

PEP-0274 introduced dict comprehensions to replace dict constructor
with a sequence of key-pairs[1], these are two benefits:
- First, it makes the code look neater.
- Second, it gains a micro-optimization.

Glance dropped python 2.6 support in Kilo, we can leverage this now.
Note: This commit doesn't handle dict constructor with kwargs.
This commit also adds a hacking rule.

[1]http://legacy.python.org/dev/peps/pep-0274/

Co-Authored-By: ChangBo Guo(gcb) <eric.guo@easystack.cn>
Co-Authored-By: Kamil Rykowski <kamil.rykowski@intel.com>
Change-Id: I0ba408f9c616dcdb09618f6256db76b9facc0c1d

HACKING.rst

@@ -22,4 +22,5 @@ glance Specific Commandments
 - [G324] Validate that LOG.error messages use _LE.
 - [G325] Validate that LOG.critical messages use _LC.
 - [G326] Validate that LOG.warning messages use _LW.
-- [G327] Prevent use of deprecated contextlib.nested
+- [G327] Prevent use of deprecated contextlib.nested
+- [G328] Must use a dict comprehension instead of a dict constructor with a sequence of key-value pairs

glance/common/client.py

@@ -413,8 +413,7 @@ class BaseClient(object):
                   names and values
         """
         to_str = encodeutils.safe_encode
-        return dict([(to_str(h), to_str(v)) for h, v in
-                     six.iteritems(headers)])
+        return {to_str(h): to_str(v) for h, v in six.iteritems(headers)}
 
     @handle_redirects
     def _do_request(self, method, url, body, headers):

glance/db/sqlalchemy/api.py

@@ -454,7 +454,7 @@ def _make_conditions_from_filters(filters, is_public=None):
             tag_filters.extend([models.ImageTag.value == tag])
             tag_conditions.append(tag_filters)
 
-    filters = dict([(k, v) for k, v in filters.items() if v is not None])
+    filters = {k: v for k, v in filters.items() if v is not None}
 
     for (k, v) in filters.items():
         key = k

glance/hacking/checks.py

@@ -55,6 +55,7 @@ log_translation_critical = re.compile(
     r"(.)*LOG\.(critical)\(\s*(_\(|'|\")")
 log_translation_warning = re.compile(
     r"(.)*LOG\.(warning)\(\s*(_\(|'|\")")
+dict_constructor_with_list_copy_re = re.compile(r".*\bdict\((\[)?(\(|\[)")
 
 
 def assert_true_instance(logical_line):
@@ -148,6 +149,13 @@ def check_no_contextlib_nested(logical_line):
         yield(0, msg)
 
 
+def dict_constructor_with_list_copy(logical_line):
+    msg = ("G328: Must use a dict comprehension instead of a dict constructor "
+           "with a sequence of key-value pairs.")
+    if dict_constructor_with_list_copy_re.match(logical_line):
+        yield (0, msg)
+
+
 def factory(register):
     register(assert_true_instance)
     register(assert_equal_type)
@@ -156,3 +164,4 @@ def factory(register):
     register(no_direct_use_of_unicode_function)
     register(validate_log_translations)
     register(check_no_contextlib_nested)
+    register(dict_constructor_with_list_copy)

glance/image_cache/drivers/sqlite.py

@@ -83,8 +83,7 @@ class SqliteConnection(sqlite3.Connection):
 
 
 def dict_factory(cur, row):
-    return dict(
-        ((col[0], row[idx]) for idx, col in enumerate(cur.description)))
+    return {col[0]: row[idx] for idx, col in enumerate(cur.description)}
 
 
 class Driver(base.Driver):

glance/registry/api/v1/images.py

@@ -466,8 +466,8 @@ class Controller(object):
         try:
             LOG.debug("Updating image %(id)s with metadata: %(image_data)r",
                       {'id': id,
-                       'image_data': dict((k, v) for k, v in image_data.items()
-                                          if k != 'locations')})
+                       'image_data': {k: v for k, v in image_data.items()
+                                      if k != 'locations'}})
             image_data = _normalize_image_location_for_db(image_data)
             if purge_props == "true":
                 purge_props = True
@@ -532,14 +532,13 @@ def make_image_dict(image):
     """
 
     def _fetch_attrs(d, attrs):
-        return dict([(a, d[a]) for a in attrs
-                    if a in d.keys()])
+        return {a: d[a] for a in attrs if a in d.keys()}
 
     # TODO(sirp): should this be a dict, or a list of dicts?
     # A plain dict is more convenient, but list of dicts would provide
     # access to created_at, etc
-    properties = dict((p['name'], p['value'])
-                      for p in image['properties'] if not p['deleted'])
+    properties = {p['name']: p['value'] for p in image['properties']
+                  if not p['deleted']}
 
     image_dict = _fetch_attrs(image, glance.db.IMAGE_ATTRS)
     image_dict['properties'] = properties

glance/registry/api/v1/members.py

@@ -356,8 +356,7 @@ def make_member_list(members, **attr_map):
     """
 
     def _fetch_memb(memb, attr_map):
-        return dict([(k, memb[v])
-                     for k, v in attr_map.items() if v in memb.keys()])
+        return {k: memb[v] for k, v in attr_map.items() if v in memb.keys()}
 
     # Return the list of members with the given attribute mapping
     return [_fetch_memb(memb, attr_map) for memb in members]

glance/tests/functional/db/base.py

@@ -287,7 +287,7 @@ class DriverTests(object):
         fixture = {'properties': {'ping': 'pong'}}
         image = self.db_api.image_update(self.adm_context, UUID1, fixture)
         expected = {'ping': 'pong', 'foo': 'bar', 'far': 'boo'}
-        actual = dict((p['name'], p['value']) for p in image['properties'])
+        actual = {p['name']: p['value'] for p in image['properties']}
         self.assertEqual(expected, actual)
         self.assertNotEqual(image['created_at'], image['updated_at'])
 
@@ -295,7 +295,7 @@ class DriverTests(object):
         fixture = {'properties': {'ping': 'pong'}}
         image = self.db_api.image_update(self.adm_context, UUID1,
                                          fixture, purge_props=True)
-        properties = dict((p['name'], p) for p in image['properties'])
+        properties = {p['name']: p for p in image['properties']}
 
         # New properties are set
         self.assertTrue('ping' in properties)

glance/tests/test_hacking.py

@@ -72,3 +72,31 @@ class HackingTestCase(utils.BaseTestCase):
 
         self.assertEqual(0, len(list(checks.check_no_contextlib_nested(
             "with foo as bar"))))
+
+    def test_dict_constructor_with_list_copy(self):
+        self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy(
+            "    dict([(i, connect_info[i])"))))
+
+        self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy(
+            "    attrs = dict([(k, _from_json(v))"))))
+
+        self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy(
+            "        type_names = dict((value, key) for key, value in"))))
+
+        self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy(
+            "   dict((value, key) for key, value in"))))
+
+        self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy(
+            "foo(param=dict((k, v) for k, v in bar.items()))"))))
+
+        self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy(
+            " dict([[i,i] for i in range(3)])"))))
+
+        self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy(
+            "  dd = dict([i,i] for i in range(3))"))))
+
+        self.assertEqual(0, len(list(checks.dict_constructor_with_list_copy(
+            "        create_kwargs = dict(snapshot=snapshot,"))))
+
+        self.assertEqual(0, len(list(checks.dict_constructor_with_list_copy(
+            "      self._render_dict(xml, data_el, data.__dict__)"))))

glance/tests/unit/test_migrations.py

@@ -276,7 +276,7 @@ class MigrationsMixin(test_migrations.WalkVersionsMixin):
         return initial_values
 
     def _check_010(self, engine, data):
-        values = dict((c, u) for c, u in data)
+        values = {c: u for c, u in data}
 
         images = db_utils.get_table(engine, 'images')
         for row in images.select().execute():
@@ -646,7 +646,7 @@ class MigrationsMixin(test_migrations.WalkVersionsMixin):
     def _check_019(self, engine, data):
         image_locations = db_utils.get_table(engine, 'image_locations')
         records = image_locations.select().execute().fetchall()
-        locations = dict([(il.image_id, il.value) for il in records])
+        locations = {il.image_id: il.value for il in records}
         self.assertEqual('http://glance.example.com',
                          locations.get('fake-19-1'))
 
@@ -937,7 +937,7 @@ class MigrationsMixin(test_migrations.WalkVersionsMixin):
         records = tasks_table.select().execute().fetchall()
         self.assertEqual(2, len(records))
 
-        tasks = dict([(t.id, t) for t in records])
+        tasks = {t.id: t for t in records}
 
         task_1 = tasks.get('task-1')
         self.assertEqual('some input', task_1.input)

tools/migrate_image_owners.py

@@ -40,7 +40,7 @@ def get_owner_map(ksclient, owner_is_tenant=True):
     else:
         entities = ksclient.users.list()
     # build mapping of (user or tenant) name to id
-    return dict([(entity.name, entity.id) for entity in entities])
+    return {entity.name: entity.id for entity in entities}
 
 
 def build_image_owner_map(owner_map, db, context):