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

Hubert Tong via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 13 05:59:44 PDT 2019


hubert.reinterpretcast added a comment.

In D67535#1669129 <https://reviews.llvm.org/D67535#1669129>, @thopre wrote:

> BTW given the size of this patch it might be worth splitting into smaller bits (eg. the type changes, the lambda one and the <> one).


I'll split out the lambda and `raise` changes first.
Then it looks like:

- Updating the type-related comparisons.
- Changing the remaining `<>` cases.
- Changing the remaining type references (if any).



================
Comment at: lnt/external/stats/pstat.py:133
 
-    if type(source) not in [ListType,TupleType]:
+    if type(source) not in [list,tuple]:
         source = [source]
----------------
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`.


================
Comment at: lnt/external/stats/pstat.py:293
                  item.append(cfcn(avgcol))
-                 if fcn1 <> None:
+                 if fcn1 != None:
                      try:
----------------
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.


================
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)))
 
----------------
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.


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


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

https://reviews.llvm.org/D67535





More information about the llvm-commits mailing list