[PATCH] D67535: [LNT] Python 3 support: Minor mechanical changes

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre added a comment.

I haven't put a comment for all occurences of the issues I pointed. Please make a scan through the patch for the whitespaces and isinstance suggestions.



================
Comment at: lnt/external/stats/pstat.py:133
 
-    if type(source) not in [ListType,TupleType]:
+    if type(source) not in [list,tuple]:
         source = [source]
----------------
I think a more pythonic change would be:

if !isinstance(source, (list, tuple))

Similarly below.


================
Comment at: lnt/external/stats/pstat.py:222
             column = abut(column,map(lambda x: x[index], listoflists))
-    elif type(cnums) == StringType:              # if an 'x[3:]' type expr.
+    elif type(cnums) == str:              # if an 'x[3:]' type expr.
         evalstring = 'map(lambda x: x'+cnums+', listoflists)'
----------------
This should be a straight isinstance. While the more complex case with several types are not caught by futurize, that one is changed as expected by futurize. I would recommend you to give it a try.


================
Comment at: lnt/external/stats/pstat.py:293
                  item.append(cfcn(avgcol))
-                 if fcn1 <> None:
+                 if fcn1 != None:
                      try:
----------------
I'm quite new to Python but I believe for comparison against None it is more usual to do "is None" or "is not None".


================
Comment at: lnt/external/stats/stats.py:515
+    if (defaultreallimits != None):
+        if type(defaultreallimits) not in [list,tuple] or len(defaultreallimits)==1: # only one limit given, assumed to be lower one & upper is calc'd
             lowerreallimit = defaultreallimits
----------------
Please add whitespace around operator while you are at it.


================
Comment at: lnt/server/reporting/runs.py:126
         else:
-            return sorted(bucket, key=lambda (_, cr, __): -abs(cr.pct_delta))
+            return sorted(bucket, key=(lambda _, cr, __: -abs(cr.pct_delta)))
 
----------------
I am not sure it is safe in this specific case to do this. With the old syntax if you passed a tuple with 3 parameters the function would behave as you would expect it (by doing tuple unpacking first) while with the new code I think it'll give an error.

Futurize gave me instead (except for the variable naming):

lambda bucket_entry: -abs(bucket_entry[1].pct_delta)

If you've checked that the function is always called as expected then the change you propose is fine (and better).


================
Comment at: lnt/server/ui/views.py:833
     # Order the plots by machine name, test name and then field.
-    graph_parameters.sort(key=lambda (m, t, f, _): (m.name, t.name, f.name, _))
+    graph_parameters.sort(key=(lambda m, t, f, _: (m.name, t.name, f.name, _)))
 
----------------
As above.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67535/new/

https://reviews.llvm.org/D67535