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

Hubert Tong via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Sep 14 10:30:38 PDT 2019


hubert.reinterpretcast marked 13 inline comments as done.
hubert.reinterpretcast added a comment.

Split out changes for lambdas (D67582 <https://reviews.llvm.org/D67582>), `raise` (D67581 <https://reviews.llvm.org/D67581>), type-related comparisons (D67587 <https://reviews.llvm.org/D67587>), and the remaining type references (also D67587 <https://reviews.llvm.org/D67587>). Will update this patch to cover the remaining `<>` cases.



================
Comment at: lnt/external/stats/pstat.py:133
 
-    if type(source) not in [ListType,TupleType]:
+    if type(source) not in [list,tuple]:
         source = [source]
----------------
hubert.reinterpretcast wrote:
> thopre wrote:
> > I think a more pythonic change would be:
> > 
> > if !isinstance(source, (list, tuple))
> > 
> > Similarly below.
> I'll look into updating to use `isinstance`.
Changed in D67587.


================
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)'
----------------
thopre wrote:
> 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.
Changed in D67587.


================
Comment at: lnt/external/stats/pstat.py:293
                  item.append(cfcn(avgcol))
-                 if fcn1 <> None:
+                 if fcn1 != None:
                      try:
----------------
cmatthews wrote:
> hubert.reinterpretcast wrote:
> > thopre wrote:
> > > 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".
> > Okay. Will do.
> space between each list elements.
Changed in D67587.


================
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)))
 
----------------
hubert.reinterpretcast wrote:
> thopre wrote:
> > 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).
> The problem you describe is likely. Will fix.
Fixed in D67582.


================
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, _)))
 
----------------
hubert.reinterpretcast wrote:
> thopre wrote:
> > As above.
> Will fix.
Fixed in D67582.


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

https://reviews.llvm.org/D67535





More information about the llvm-commits mailing list