Merge pull request #62 from zopefoundation/code-cleaup-keys

Code cleanup: do not call dict.keys

CHANGES.rst

@@ -4,7 +4,9 @@ Products.CMFCore Changelog
 2.4.0b7 (unreleased)
 --------------------
 
-- Nothing changed yet.
+- Code cleanup: avoid to call the ``keys`` method on dictionary
+  when not needed
+  [ale-rt]
 
 
 2.4.0b6 (2018-12-14)

Products/CMFCore/CMFCatalogAware.py

@@ -229,7 +229,7 @@ def opaqueItems(self):
         # Loop over all attributes and add those to 'items'
         # implementing 'ICallableOpaqueItem'.
         self_base = aq_base(self)
-        for name in self_base.__dict__.keys():
+        for name in self_base.__dict__:
             obj = getattr(self, name)
             if ICallableOpaqueItem.providedBy(obj):
                 items.append((obj.getId(), obj))

Products/CMFCore/CatalogTool.py

@@ -118,7 +118,7 @@ def allowedRolesAndUsers(self):
                     allowed['user:' + user] = 1
         if 'Owner' in allowed:
             del allowed['Owner']
-        return list(allowed.keys())
+        return list(allowed)
 
     def cmf_uid(self):
         """
@@ -362,8 +362,7 @@ def _reindexObject(self, object, idxs=[], update_metadata=1, uid=None):
             uid = self.__url(object)
         if idxs != []:
             # Filter out invalid indexes.
-            valid_indexes = self._catalog.indexes.keys()
-            idxs = [i for i in idxs if i in valid_indexes]
+            idxs = [i for i in idxs if i in self._catalog.indexes]
         self.catalog_object(object, uid, idxs, update_metadata)
 
 

Products/CMFCore/DirectoryView.py

@@ -277,7 +277,7 @@ def prepareContents(self, registry):
                     # FS-based security
                     permissions = metadata.getSecurity()
                     if permissions is not None:
-                        for name in permissions.keys():
+                        for name in permissions:
                             acquire, roles = permissions[name]
                             try:
                                 ob.manage_permission(name, roles, acquire)
@@ -347,7 +347,7 @@ def getDirectoryInfo(self, reg_key):
         return self._directories.get(reg_key, None)
 
     def listDirectories(self):
-        dirs = sorted(self._directories.keys())
+        dirs = sorted(self._directories)
         return dirs
 
 

Products/CMFCore/MemberDataTool.py

@@ -103,8 +103,8 @@ def getMemberDataContents(self):
         mtool = getUtility(IMembershipTool)
         members = self._members
         user_list = mtool.listMemberIds()
-        member_list = members.keys()
-        member_count = len(members)
+        member_list = list(members)
+        member_count = len(member_list)
         orphan_count = 0
 
         for member in member_list:
@@ -130,7 +130,7 @@ def searchMemberData(self, search_param, search_term, attributes=()):
         if search_param == 'username':
             search_param = 'id'
 
-        for user_id in self._members.keys():
+        for user_id in self._members:
             u = mtool.getMemberById(user_id)
 
             if u is not None:
@@ -160,7 +160,7 @@ def searchMemberDataContents(self, search_param, search_term):
 
         mtool = getUtility(IMembershipTool)
 
-        for member_id in self._members.keys():
+        for member_id in self._members:
 
             user_wrapper = mtool.getMemberById(member_id)
 
@@ -182,7 +182,7 @@ def pruneMemberDataContents(self):
         members = self._members
         user_list = mtool.listMemberIds()
 
-        for member_id in list(members.keys()):
+        for member_id in list(members):
             if member_id not in user_list:
                 del members[member_id]
 

Products/CMFCore/MembershipTool.py

@@ -144,7 +144,7 @@ def wrapUser(self, u, wrap_anon=0):
 
         # Apply any role mapping if we have it
         if hasattr(self, 'role_map'):
-            for portal_role in self.role_map.keys():
+            for portal_role in self.role_map:
                 if (self.role_map.get(portal_role) in u.roles and
                         portal_role not in u.roles):
                     u.roles.append(portal_role)

Products/CMFCore/SkinsTool.py

@@ -122,7 +122,7 @@ def manage_skinLayers(self, chosen=(), add_skin=0, del_skin=0,
                 del sels[name]
 
         if REQUEST is not None:
-            for key in sels.keys():
+            for key in sels:
                 fname = 'skinpath_%s' % key
                 val = REQUEST[fname]
 
@@ -291,7 +291,7 @@ def getSkinSelections(self):
         """ Get the sorted list of available skin names.
         """
         sels = self._getSelections()
-        rval = sorted(sels.keys())
+        rval = sorted(sels)
         return rval
 
     @security.protected(View)

Products/CMFCore/TypesTool.py

@@ -790,7 +790,7 @@ def listContentTypes(self, container=None, by_metatype=0):
             if name:
                 typenames[name] = None
 
-        result = sorted(typenames.keys())
+        result = sorted(typenames)
         return result
 
     @security.public
@@ -844,9 +844,9 @@ def listMethodAliasKeys(self):
         _dict = {}
         for ti in self.listTypeInfo():
             aliases = ti.getMethodAliases()
-            for k in aliases.keys():
+            for k in aliases:
                 _dict[k] = 1
-        rval = sorted(_dict.keys())
+        rval = sorted(_dict)
         return rval
 
 

Products/CMFCore/WorkflowTool.py

@@ -598,7 +598,7 @@ def _reindexWorkflowVariables(self, ob):
         if hasattr(aq_base(ob), 'reindexObject'):
             # XXX We only need the keys here, no need to compute values.
             mapping = self.getCatalogVariablesFor(ob) or {}
-            vars = mapping.keys()
+            vars = list(mapping)
             ob.reindexObject(idxs=vars)
 
         # Reindex security of subobjects.

Products/CMFCore/tests/test_CachingPolicyManager.py

@@ -171,7 +171,7 @@ def test_viewPredicateMiss(self):
 
     def test_kwPredicate(self):
         policy = self._makePolicy('kwPredicate',
-                                  predicate='python:"foo" in keywords.keys()')
+                                  predicate='python:"foo" in keywords')
         context = self._makeContext(foo=1)
         headers = policy.getHeaders(context)
 
@@ -181,7 +181,7 @@ def test_kwPredicate(self):
 
     def test_kwPredicateMiss(self):
         policy = self._makePolicy('kwPredicateMiss',
-                                  predicate='python:"foo" in keywords.keys()')
+                                  predicate='python:"foo" in keywords')
         context = self._makeContext(bar=1)
         headers = policy.getHeaders(context)
 
@@ -479,7 +479,7 @@ def test_reorder(self):
 
         for policy_id in policy_ids:
             mgr._addPolicy(policy_id,
-                           'python:"%s" in keywords.keys()' % policy_id,
+                           'python:"%s" in keywords' % policy_id,
                            None, 0, 0, 0, 0, '', '')
 
         ids = tuple([x[0] for x in mgr.listPolicies()])
@@ -500,7 +500,7 @@ def _makeOneWithPolicies(self):
 
         for policy_id, max_age_secs in policy_tuples:
             mgr._addPolicy(policy_id,
-                           'python:"%s" in keywords.keys()' % policy_id,
+                           'python:"%s" in keywords' % policy_id,
                            None, max_age_secs, 0, 0, 0, '', '')
 
         return mgr
@@ -894,7 +894,7 @@ def test_subtemplate_cpm_1(self):
         portal.content()
 
         # no headers should be added by the CPM if all is well
-        headers = [x.lower() for x in self.RESPONSE.headers.keys()]
+        headers = {x.lower() for x in self.RESPONSE.headers}
         self.assertFalse('x-cache-headers-set-by' in headers)
         self.assertFalse('vary' in headers)
 
@@ -935,7 +935,7 @@ def test_subtemplate_cpm_2(self):
         portal.content()
 
         # no headers should be added by the CPM if all is well
-        headers = [x.lower() for x in self.RESPONSE.headers.keys()]
+        headers = {x.lower() for x in self.RESPONSE.headers}
         self.assertFalse('x-cache-headers-set-by' in headers)
         self.assertFalse('vary' in headers)
 
@@ -980,7 +980,7 @@ def test_subtemplate_cpm_3(self):
         portal.doc1()
 
         # no headers should be added by the CPM if all is well
-        headers = [x.lower() for x in self.RESPONSE.headers.keys()]
+        headers = {x.lower() for x in self.RESPONSE.headers}
         self.assertFalse('x-cache-headers-set-by' in headers)
         self.assertFalse('vary' in headers)
 
@@ -1025,7 +1025,7 @@ def test_mixed_subtemplate_cpm(self):
         portal.doc1()
 
         # no headers should be added by the CPM if all is well
-        headers = [x.lower() for x in self.RESPONSE.headers.keys()]
+        headers = {x.lower() for x in self.RESPONSE.headers}
         self.assertFalse('x-cache-headers-set-by' in headers)
         self.assertFalse('vary' in headers)
 
@@ -1087,7 +1087,7 @@ def test_fireForSubtemplates(self):
         self.portal.doc1.dummy_view()
 
         # no headers should be added by the CPM if all is well
-        headers = [x.lower() for x in self.RESPONSE.headers.keys()]
+        headers = {x.lower() for x in self.RESPONSE.headers}
         self.assertFalse('x-cache-headers-set-by' in headers)
         self.assertFalse('vary' in headers)
 
@@ -1238,7 +1238,7 @@ def test_no_association(self):
         self.portal.doc1()
 
         # no headers should be added by the CPM if all is well
-        headers = [x.lower() for x in self.RESPONSE.headers.keys()]
+        headers = {x.lower() for x in self.RESPONSE.headers}
         self.assertFalse('x-cache-headers-set-by' in headers)
         self.assertFalse('vary' in headers)
 
@@ -1252,7 +1252,7 @@ def test_unsuitable_association(self):
         doc2()
 
         # no headers should be added by the CPM if all is well
-        headers = [x.lower() for x in self.RESPONSE.headers.keys()]
+        headers = {x.lower() for x in self.RESPONSE.headers}
         self.assertFalse('x-cache-headers-set-by' in headers)
         self.assertFalse('vary' in headers)
 

Products/CMFCore/tests/test_DirectoryView.py

@@ -178,7 +178,7 @@ def tearDown(self):
         # This is nasty, but there is no way to unregister anything
         # right now...
         metatype_registry = DirectoryView._dirreg._meta_types
-        if 'FOLDER' in metatype_registry.keys():
+        if 'FOLDER' in metatype_registry:
             del metatype_registry['FOLDER']
         FSDVTest.tearDown(self)
 

Products/CMFCore/tests/test_FSDTMLMethod.py

@@ -73,8 +73,8 @@ def test_caching(self):
         obj = obj.__of__(self.app)
         obj(self.app, self.REQUEST, self.RESPONSE)
         self.assertTrue(len(self.RESPONSE.headers) >= original_len + 2)
-        self.assertTrue('foo' in self.RESPONSE.headers.keys())
-        self.assertTrue('bar' in self.RESPONSE.headers.keys())
+        self.assertTrue('foo' in self.RESPONSE.headers)
+        self.assertTrue('bar' in self.RESPONSE.headers)
 
     def test_ownership(self):
         script = self._makeOne('testDTML', 'testDTML.dtml')

Products/CMFCore/tests/test_FSFile.py

@@ -175,8 +175,8 @@ def test_caching(self):
         obj.index_html(self.REQUEST, self.RESPONSE)
         headers = self.RESPONSE.headers
         self.assertTrue(len(headers) >= original_len + 3)
-        self.assertTrue('foo' in headers.keys())
-        self.assertTrue('bar' in headers.keys())
+        self.assertTrue('foo' in headers)
+        self.assertTrue('bar' in headers)
         self.assertEqual(headers['test_path'], '/test_file')
 
     def test_forced_content_type(self):

Products/CMFCore/tests/test_FSImage.py

@@ -165,8 +165,8 @@ def test_caching(self):
         obj.index_html(self.REQUEST, self.RESPONSE)
         headers = self.RESPONSE.headers
         self.assertTrue(len(headers) >= original_len + 3)
-        self.assertTrue('foo' in headers.keys())
-        self.assertTrue('bar' in headers.keys())
+        self.assertTrue('foo' in headers)
+        self.assertTrue('bar' in headers)
         self.assertEqual(headers['test_path'], '/test_image')
 
     def test_index_html_with_304_and_caching(self):
@@ -190,8 +190,8 @@ def test_index_html_with_304_and_caching(self):
 
         headers = self.RESPONSE.headers
         self.assertTrue(len(headers) >= original_len + 3)
-        self.assertTrue('foo' in headers.keys())
-        self.assertTrue('bar' in headers.keys())
+        self.assertTrue('foo' in headers)
+        self.assertTrue('bar' in headers)
         self.assertEqual(headers['test_path'], '/test_image')
 
     def test_tag_with_acquired_clashing_attrs(self):

Products/CMFCore/tests/test_FSPageTemplate.py

@@ -142,8 +142,8 @@ def test_caching(self):
         obj = obj.__of__(self.app)
         obj()
         self.assertTrue(len(self.RESPONSE.headers) >= original_len + 2)
-        self.assertTrue('foo' in self.RESPONSE.headers.keys())
-        self.assertTrue('bar' in self.RESPONSE.headers.keys())
+        self.assertTrue('foo' in self.RESPONSE.headers)
+        self.assertTrue('bar' in self.RESPONSE.headers)
 
     def test_pt_properties(self):
         script = self._makeOne('testPT', 'testPT.pt')

Products/CMFCore/tests/test_FSReSTMethod.py

@@ -113,8 +113,8 @@ def test_caching(self):
         obj = obj.__of__(self.app)
         obj(self.REQUEST, self.RESPONSE)
         self.assertTrue(len(self.RESPONSE.headers) >= original_len + 2)
-        self.assertTrue('foo' in self.RESPONSE.headers.keys())
-        self.assertTrue('bar' in self.RESPONSE.headers.keys())
+        self.assertTrue('foo' in self.RESPONSE.headers)
+        self.assertTrue('bar' in self.RESPONSE.headers)
 
     def test_ownership(self):
         script = self._makeOne('testReST', 'testReST.rst')

Products/CMFCore/tests/test_FSSTXMethod.py

@@ -140,8 +140,8 @@ def test_caching(self):
         obj = obj.__of__(self.app)
         obj(self.REQUEST, self.RESPONSE)
         self.assertTrue(len(self.RESPONSE.headers) >= original_len + 2)
-        self.assertTrue('foo' in self.RESPONSE.headers.keys())
-        self.assertTrue('bar' in self.RESPONSE.headers.keys())
+        self.assertTrue('foo' in self.RESPONSE.headers)
+        self.assertTrue('bar' in self.RESPONSE.headers)
 
     def test_ownership(self):
         script = self._makeOne('testSTX', 'testSTX.stx')

Products/CMFCore/tests/test_WorkflowTool.py

@@ -274,7 +274,7 @@ def test_content_own_chain(self):
 
         vars = tool.getCatalogVariablesFor(dummy)
         self.assertEqual(len(vars), 1)
-        self.assertTrue('dummy' in vars.keys())
+        self.assertTrue('dummy' in vars)
         self.assertTrue('a: dummy' in vars.values())
 
     def test_setChainForPortalTypes(self):
@@ -308,7 +308,7 @@ def test_getCatalogVariablesFor(self):
 
         vars = tool.getCatalogVariablesFor(dummy)
         self.assertEqual(len(vars), 1)
-        self.assertTrue('dummy' in vars.keys())
+        self.assertTrue('dummy' in vars)
         self.assertTrue('a: dummy' in vars.values())
 
     def test_getInfoFor(self):

Products/CMFCore/zcml.py

@@ -85,7 +85,7 @@ def registerDirectory(_context, name, directory=None, recursive=False,
 def cleanUp():
     global _directory_regs
     for reg_key in _directory_regs:
-        for key in list(_dirreg._directories.keys()):
+        for key in list(_dirreg._directories):
             if key.startswith(reg_key):
                 del _dirreg._directories[key]
     _directory_regs = []