py2/py3: dict_key does not have sort().

product/ERP5/Document/BusinessTemplate.py

@@ -1193,9 +1193,7 @@ class ObjectTemplateItem(BaseTemplateItem):
 
   def _getObjectKeyList(self):
     # sort to add objects before their subobjects
-    keys = ensure_list(self._objects.keys())
-    keys.sort()
-    return keys
+    return sorted(self._objects.keys())
 
   def unindexBrokenObject(self, item_path):
     """
@@ -1673,11 +1671,9 @@ class PathTemplateItem(ObjectTemplateItem):
     if object_path is not None:
       object_keys = [object_path]
     else:
-      object_keys = ensure_list(self._path_archive.keys())
-    object_keys.sort()
-    object_keys.reverse()
+      object_keys = self._path_archive.keys()
     p = context.getPortalObject()
-    for path in object_keys:
+    for path in sorted(object_keys, reverse=True):
       try:
         path_list = self._resolvePath(p, [], path.split('/'))
       except AttributeError:
@@ -1728,9 +1724,7 @@ class PathTemplateItem(ObjectTemplateItem):
   def build(self, context, **kw):
     BaseTemplateItem.build(self, context, **kw)
     p = context.getPortalObject()
-    keys = ensure_list(self._path_archive.keys())
-    keys.sort()
-    for path in keys:
+    for path in sorted(self._path_archive.keys()):
       include_subobjects = 0
       if path.endswith("**"):
         include_subobjects = 1
@@ -2081,14 +2075,11 @@ class RegisteredSkinSelectionTemplateItem(BaseTemplateItem):
   # Function to generate XML Code Manually
   def generateXml(self, path=None):
     xml_data = '<registered_skin_selection>'
-    keys = ensure_list(self._objects.keys())
-    keys.sort()
-    for key in keys:
-      skin_selection_list = self._objects[key]
+    for key in sorted(self._objects.keys()):
       xml_data += '\n <skin_folder_selection>'
       xml_data += '\n  <skin_folder>%s</skin_folder>' % key
       xml_data += '\n  <skin_selection>%s</skin_selection>' \
-                      % ','.join(sorted(skin_selection_list))
+                      % ','.join(sorted(self._objects[key]))
       xml_data += '\n </skin_folder_selection>'
     xml_data += '\n</registered_skin_selection>'
     return xml_data
@@ -2459,7 +2450,7 @@ class PortalTypeTemplateItem(ObjectTemplateItem):
 
   def _getObjectKeyList(self):
     # Sort portal types to install according to their dependencies
-    object_key_list = ensure_list(self._objects.keys())
+    object_key_list = self._objects.keys()
     path_dict = dict(x.split('/')[1:] + [x] for x in object_key_list)
     cache = {}
     def solveDependency(path):
@@ -2480,8 +2471,7 @@ class PortalTypeTemplateItem(ObjectTemplateItem):
           return 0, path
         cache[path] = score = depend and 1 + solveDependency(depend)[0] or 0
       return score, path
-    object_key_list.sort(key=solveDependency)
-    return object_key_list
+    return sorted(object_key_list, key=solveDependency)
 
   # XXX : this method is kept temporarily, but can be removed once all bt5 are
   # re-exported with separated workflow-chain information
@@ -2561,14 +2551,11 @@ class PortalTypeWorkflowChainTemplateItem(BaseTemplateItem):
   # Function to generate XML Code Manually
   def generateXml(self, path=None):
     xml_data = '<workflow_chain>'
-    key_list = ensure_list(self._objects.keys())
-    key_list.sort()
-    for key in key_list:
-      workflow_list = self._objects[key]
+    for key in sorted(self._objects.keys()):
       xml_data += '\n <chain>'
       xml_data += '\n  <type>%s</type>' %(key,)
       xml_data += '\n  <workflow>%s</workflow>' %(
-        self._chain_string_separator.join(sorted(workflow_list)))
+        self._chain_string_separator.join(sorted(self._objects[key])))
       xml_data += '\n </chain>'
     xml_data += '\n</workflow_chain>'
     return xml_data
@@ -2778,9 +2765,7 @@ class PortalTypeAllowedContentTypeTemplateItem(BaseTemplateItem):
   # Function to generate XML Code Manually
   def generateXml(self, path=None):
     xml_data = '<%s>' %(self.xml_tag,)
-    key_list = ensure_list(self._objects.keys())
-    key_list.sort()
-    for key in key_list:
+    for key in sorted(self._objects.keys()):
       id_value = key.replace('%s/' % self.class_property, '')
       allowed_item_list = sorted(self._objects[key])
       xml_data += '\n <portal_type id="%s">' % (id_value)
@@ -3668,9 +3653,7 @@ class SitePropertyTemplateItem(BaseTemplateItem):
     if len(self._objects) == 0:
       return
     xml_data = '<site_property>'
-    keys = ensure_list(self._objects.keys())
-    keys.sort()
-    for path in keys:
+    for path in sorted(self._objects.keys()):
       xml_data += self.generateXml(path)
     xml_data += '\n</site_property>'
     bta.addObject(xml_data, name='properties', path=self.__class__.__name__)
@@ -3736,9 +3719,7 @@ class ModuleTemplateItem(BaseTemplateItem):
     if len(self._objects) == 0:
       return
     path = self.__class__.__name__
-    keys = ensure_list(self._objects.keys())
-    keys.sort()
-    for key in keys:
+    for key in sorted(self._objects.keys()):
       # export modules one by one
       xml_data = self.generateXml(path=key)
       bta.addObject(xml_data, name=key, path=path)

product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/Base_callDialogMethod.py

@@ -132,8 +132,7 @@ if len(listbox_id_list):
     listbox_line_list = []
     listbox = kw[listbox_id]
     listbox_keys = listbox.keys()
-    listbox_keys.sort()
-    for key in listbox_keys:
+    for key in sorted(listbox_keys):
       listbox_line = listbox[key]
       listbox_line['listbox_key'] = key
       listbox_line_list.append(listbox_line)

product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/Base_viewDict.py

@@ -14,9 +14,8 @@ from pprint import pformat
 ret = '<html><body><table width=100%>\n'
 
 property_dict = context.showDict().items()
-property_dict.sort()
 i = 0
-for k,v in property_dict:
+for k,v in sorted(property_dict):
   if (i % 2) == 0:
     c = '#88dddd'
   else:

product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/BusinessTemplate_getDiffObjectList.py

@@ -36,11 +36,10 @@ else:
   modified_object_list = getModifiedObjectList(bt1, bt2)
 
 keys = modified_object_list.keys()
-keys.sort()
 
 i = 0
 object_list = []
-for object_id in keys:    
+for object_id in sorted(keys):
   object_state, object_class = modified_object_list[object_id]
   line = newTempBase(context, 'tmp_install_%s' %(str(i)))
   line.edit(object_id=object_id, object_state=object_state, object_class=object_class, bt1=bt1.getId(), bt2=bt2.getId())

product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/BusinessTemplate_getModifiedObject.py

@@ -22,7 +22,6 @@ getModifiedObjectList = CachingMethod(getModifiedObjectList,
 
 modified_object_list = getModifiedObjectList(context)
 keys = ensure_list(modified_object_list.keys())
-keys.sort()
 
 no_backup_list = ['Action', 'SiteProperty', 'Module', 'Document',
                   'PropertySheet', 'Extension', 'Test', 'Product', 'Role',
@@ -43,7 +42,7 @@ save_and_remove_title = Base_translateString('Backup And Remove')
 
 i = 0
 object_list = []
-for object_id in keys:
+for object_id in sorted(keys):
   object_state, object_class = modified_object_list[object_id]
   line = newTempBase(context, 'tmp_install_%s' %(str(i)))
   if object_state == 'New':

product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/Folder_modifyWorkflowStatus.py

@@ -45,8 +45,7 @@ for form in (real_form, target_form):
     listbox_line_list = []
     listbox = getattr(request,'listbox',None) # XXX: hardcoded field name
     listbox_keys = listbox.keys()
-    listbox_keys.sort()
-    for key in listbox_keys:
+    for key in sorted(listbox_keys):
       listbox_line = listbox[key]
       listbox_line['listbox_key'] = key
       listbox_line_list.append(listbox[key])

product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/ListBox_initializeFastInput.py

@@ -19,10 +19,9 @@ if hasattr(request, listbox_id):
   # initialize the listbox 
   listbox=request[listbox_id]
 
-  keys_list = listbox.keys()
+  keys_list = sorted(listbox.keys(), key=int)
 
   if keys_list != []:
-    keys_list.sort(key=int)
     first_empty_line_id = int(keys_list[-1])+1
 
   for i in keys_list:

product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/TemplateTool_getModifiedObjectList.py

@@ -46,8 +46,7 @@ save_and_remove_title = Base_translateString('Backup And Remove')
 for bt in bt_id_list:
   bt_title, modified_object_list = bt_object_dict[bt]
   keys = modified_object_list.keys()
-  keys.sort()
-  for i, object_id in enumerate(keys):    
+  for i, object_id in enumerate(sorted(keys)):
     object_state, object_class = modified_object_list[object_id]
     object_id = bt+'|'+object_id
     line = newTempBase(context, 'tmp_install_%s' % i)

product/ERP5Type/Tool/WorkflowTool.py

@@ -691,8 +691,7 @@ def getValidCriterionDict(worklist_match_dict, sql_catalog,
 def updateWorklistSetDict(worklist_set_dict, workflow_worklist_key, valid_criterion_dict):
   worklist_set_dict_key = valid_criterion_dict.keys()
   if len(worklist_set_dict_key):
-    worklist_set_dict_key.sort()
-    worklist_set_dict_key = tuple(worklist_set_dict_key)
+    worklist_set_dict_key = tuple(sorted(worklist_set_dict_key))
     if worklist_set_dict_key not in worklist_set_dict:
       worklist_set_dict[worklist_set_dict_key] = {}
     worklist_set_dict[worklist_set_dict_key]\

product/ERP5Type/XMLExportImport/__init__.py

@@ -143,16 +143,14 @@ def Base_asXML(object, root=None):
   # We have to describe the workflow history
   if getattr(self, 'workflow_history', None) is not None:
     workflow_list = self.workflow_history
-    workflow_list_keys = ensure_list(workflow_list.keys())
-    workflow_list_keys.sort() # Make sure it is sorted
+    workflow_list_keys = workflow_list.keys()
 
-    for workflow_id in workflow_list_keys:
+    for workflow_id in sorted(workflow_list_keys):  # Make sure it is sorted
       for workflow_action in workflow_list[workflow_id]:
         workflow_node = SubElement(object, 'workflow_action',
                                    attrib=dict(workflow_id=workflow_id))
-        workflow_variable_list = ensure_list(workflow_action.keys())
-        workflow_variable_list.sort()
-        for workflow_variable in workflow_variable_list:
+        workflow_variable_list = workflow_action.keys()
+        for workflow_variable in sorted(workflow_variable_list):
           variable_type = "string" # Somewhat bad, should find a better way
           if workflow_variable.find('time') >= 0:
             variable_type = "date"

product/Localizer/MessageCatalog.py

@@ -644,9 +644,8 @@ class MessageCatalog(LanguageManager, ObjectManager, SimpleItem):
             return x
 
         # Generate sorted msgids to simplify diffs
-        dkeys = ensure_list(d.keys())
-        dkeys.sort()
-        for k in dkeys:
+        dkeys = d.keys()
+        for k in sorted(dkeys):
             r.append('msgid "%s"' % backslashescape(k))
             v = d[k]
             r.append('msgstr "%s"' % backslashescape(v))

product/MailTemplates/BaseMailTemplate.py

@@ -118,9 +118,8 @@ class BaseMailTemplate:
         # we want to have it stored in ERP5, for mail threading
         headers['Message-ID'] = make_msgid()
         # turn headers into an ordered list for predictable header order
-        keys = ensure_list(headers.keys())
-        keys.sort()
-        return msg,values,[(key,headers[key]) for key in keys]
+        keys = headers.keys()
+        return msg,values,[(key,headers[key]) for key in sorted(keys)]
 
     security.declarePrivate('_send')
     def _send(self,mfrom,mto,msg):

product/MailTemplates/FSMailTemplate.py

@@ -36,8 +36,7 @@ class FSMailTemplate(BaseMailTemplate,FSPageTemplate):
         obj.content_type = self.content_type
         if self._properties:
             keys = self._properties.keys()
-            keys.sort()
-            for id in keys:
+            for id in sorted(keys):
                 if id not in ('mailhost','content_type'):
                     obj.manage_addProperty(id,self._properties[id],'string')
         return obj

[Vincent Pelletier]

Isn't .keys() useless here ?

[Jérome Perrin]

Yes, it's not following yet the rules that we were discussing on forum, this is still a case of "we don't know if it's python3 compatible because it's using .keys()" so technically speaking it's not in the wrong direction, but the dict story is not finished yet

[Vincent Pelletier]

(ditto)

[Vincent Pelletier]

(ditto)

EDIT: actually, given that the loop is also accessing values, I think the following is more readable:

for key, value in sorted(self._objects.items(), key=lambda x: x[0]):
  ...
    % ','.join(sorted(value))
  ...

The lambda can be simplified into an itemgetter, or could be just discarded entirely (although this does not express the original intent).

[Jérome Perrin]

yesterday when I posted

... unless maybe the loop is doing something like this:

       for k in sorted(d):
         v = d[k]
         ...

in such cases we can probably switch to for k, v in sorted(six.iteritems(d)) at the same time.

I was thinking of this exact diff :)

[Jérome Perrin]

I posted before seeing the edit. I believe we should use six.iteritems(self._objects) and not .items() to mark this as python3 compatible already.

Also I'm wondering if it is really needed to use a lambda ? can't we expect that sorting the items produces the same result ?

[Vincent Pelletier]

I posted before seeing the edit. I believe we should use six.iteritems(self._objects) and not .items() to mark this as python3 compatible already.

Ah right, I forgot this again. This is better.

Also I'm wondering if it is really needed to use a lambda ? can't we expect that sorting the items produces the same result ?

This is what I mean by (especially the last part):

The lambda can be simplified into an itemgetter, or could be just discarded entirely (although this does not express the original intent).

[Jérome Perrin]

This "use six.iter*() rule everywhere is a bit counter intuitive and easy to forget. We should try to cover it by the coding style test.

Sorry about the lambda part, I understand now, I must have been reading too fast.

[Vincent Pelletier]

(ditto, with the "EDIT" also applying here)

[Vincent Pelletier]

(ditto without the EDIT)

[Vincent Pelletier]

(ditto without the EDIT)

[Vincent Pelletier]

(ditto without the EDIT)

[Vincent Pelletier]

listbox_keys is not used later, so this can be simplified, and previous comments taken into account.

[Vincent Pelletier]

property_dict is not used elsewherem so this can be simplified.

Also, the coding style on the same could be fixed.

[Vincent Pelletier]

keys is not used elsewhere, so this can be simplified.

EDIT: also, enumerate can be used to simplify the code further.

[Vincent Pelletier]

(same as BusinessTemplate_getDiffObjectList.py)

[Vincent Pelletier]

listbox_keys is not used elsewhere, so this can be simplified.

[Vincent Pelletier]

.keys() is unnecessary.

[Vincent Pelletier]

keys is not used elsewhere, this can be simplified further.