[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