[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