[PATCH] D67817: [LNT] Python 3 support: adapt to map returning an iterator
Hubert Tong via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 24 20:49:29 PDT 2019
hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast marked an inline comment as done.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.
LGTM with comments on further changes that could be made.
================
Comment at: lnt/external/stats/pstat.py:965
if row1.dtype.char=='O' or row2.dtype=='O':
- cmpvect = N.logical_not(abs(N.array(map(cmp,row1,row2)))) # cmp fcn gives -1,0,1
+ cmpvect = N.logical_not(abs(N.array(list(map(cmp, row1, row2))))) # cmp fcn gives -1,0,1
else:
----------------
Not the subject of this patch, but the `cmp` might be a problem.
================
Comment at: lnt/external/stats/pstat.py:1025
for unq in uniques: # NOTE: cmp --> 0=same, -1=<, 1=>
- test = N.sum(abs(N.array(map(cmp,item,unq))))
+ test = N.sum(abs(N.array(list(map(cmp, item, unq)))))
if test == 0: # if item identical to any 1 row in uniques
----------------
Ditto.
================
Comment at: lnt/external/stats/stats.py:1569
alldata = []
- tmp = map(N.array,lists)
- means = map(amean,tmp)
- vars = map(avar,tmp)
- ns = map(len,lists)
+ tmp = map(N.array, lists)
+ means = list(map(amean, tmp))
----------------
thopre wrote:
> hubert.reinterpretcast wrote:
> > `tmp` is consumed twice immediately below, so the `list` application would still be appropriate if we aren't removing this block altogether.
> Yes but map accepts an iterable so if the only uses are below (as the name tmp would imply) that change should be fine. I grepped LNT around for vars and means and couldn't find use elsewhere and git history isn't very informative. I've thus decided to remove the entire block.
I agree with the removal of the block. On the now-moot point (in case it comes up in the future), by "consume" I did mean //destructively// consume. That is:
```
tmp = map(str, [0, 1])
a = list(tmp)
b = list(tmp) # Empty list!
```
================
Comment at: lnt/external/stats/stats.py:1266
args = list(args)
n = [0]*len(args)
all = []
----------------
Another opportunity to do clean-up in another patch. It seems this is some attempt to preallocate `n` (and it happens in a few other places), but I haven't found anything that indicates that this isn't just dead code.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67817/new/
https://reviews.llvm.org/D67817
More information about the llvm-commits
mailing list