[PATCH] D67814: [LNT] Python 3 support: adapt to dict method returning views

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: examples/run_to_csv.py:10
 
-titles = data['tests'].itervalues().next().keys()
+titles = iter(data['tests'].values()).next().keys()
 titles.insert(0, titles.pop(titles.index("name")))
----------------
I believe this use of `next()` is one of the ones that need to be replaced in the style of D67812. This file seems to be unmaintained and untested. None of the `*.json` files in the `tests/` directory seems to match the format this script expects. I would suggest deleting this file.


================
Comment at: examples/run_to_csv.py:15
 
-for i, result in data['tests'].items():
+for i, result in list(data['tests'].items()):
 
----------------
I believe the change on this line is unnecessary.


================
Comment at: lnt/lnttool/admin.py:32
         self._try_load_config(args['config'])
-        for key, value in args.items():
+        for key, value in list(args.items()):
             self._set(key, value)
----------------
I believe the change on this line is unnecessary.


================
Comment at: lnt/lnttool/admin.py:50
             config = yaml.load(open(filename))
-            for key, value in config.items():
+            for key, value in list(config.items()):
                 self._set(key, value)
----------------
I believe the change on this line is unnecessary. I'm not marking further instances, but we generally shouldn't form a list just to iterate.


================
Comment at: lnt/server/db/rules/rule_update_fixed_regressions.py:78
     # Order regressions by FC end order.
-    ordered = sorted(regression_newest_change.items(), key=lambda x: x[1])
+    ordered = sorted(list(regression_newest_change.items()), key=lambda x: x[1])
     to_move = ordered[0:(-1 * num_to_keep)]
----------------
I think this creates a list and then creates a sorted copy. I am inclined to leave the line unchanged (and for similar instances elsewhere in this patch).


================
Comment at: lnt/server/ui/views.py:958
 
         data.sort(key=lambda sample: convert_revision(sample[0], cache=revision_cache))
 
----------------
Is this use of `sort` something that would be considered within the scope of D67809?


================
Comment at: lnt/server/ui/views.py:1017
         # Calculate geomean of each revision.
-        data = multidict.multidict(
-                    ((rev, date), val) for val, rev, date in q).items()
+        data = list(multidict.multidict(
+                    ((rev, date), val) for val, rev, date in q).items())
----------------
This also seems to be used just to iterate through once.


================
Comment at: lnt/server/ui/views.py:1322
     # Get a sorted list of recent machines.
-    recent_machines = sorted(recent_runs_by_machine.keys(),
+    recent_machines = sorted(list(recent_runs_by_machine.keys()),
                              key=lambda m: m.name)
----------------
Another `sorted` over a fresh list.


================
Comment at: lnt/testing/profile/profilev2impl.py:270
+        for f in list(impl.getFunctions().values()):
+            keys += list(f['counters'].keys())
         keys = sorted(set(keys))
----------------
`keys.extend(f['counters'].keys())`?


================
Comment at: lnt/testing/profile/profilev2impl.py:317
             for counters, address, text in self.impl.getCodeForFunction(fname):
                 for k in sorted(all_counters):
                     writeFloat(fobj, counters.get(k, 0))
----------------
I guess the `sorted()` should go in place of `list()` where the latter is added a few lines up.


================
Comment at: lnt/tests/nt.py:1483
                          if x.is_rerunable()]
     rerunable_benches.sort(key=lambda x: x.name)
     # Now lets do the reruns.
----------------
Another call to `sort`. Maybe use something like the following instead?

```
sorted(filter(lambda bench: bench.is_rerunable(),
              collated_results.values()),
       key=lambda bench: bench.name)
```


================
Comment at: lnt/tests/nt.py:1488
     logger.info(summary.format(len(rerunable_benches),
-                               len(collated_results.values())))
+                               len(list(collated_results.values()))))
 
----------------
`len(collated_results)`


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

https://reviews.llvm.org/D67814