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

[thopre]

Summary

Adapt calls to dictionary's keys(), items() and values() to them
returning a view in Python 3 when necessary, undoing unneeded changes
in lnt/testing/__init__.py. Also replace calls to their iter relatives
to use iter() instead and calls to the their views relatives to use the
plains .keys(), .items() and .values(). This was produced by running
futurize's stage2 lib2to3.fixes.fix_dict with manual intervention to
clean up code and remove unnecessary list() wrapping.

[hubert.reinterpretcast]

examples/run_to_csv.py

10 ↗

(On Diff #220972)

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.

15 ↗

(On Diff #220972)

I believe the change on this line is unnecessary.

lnt/lnttool/admin.py

32 ↗

(On Diff #220972)

I believe the change on this line is unnecessary.

50 ↗

(On Diff #220972)

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.

lnt/server/db/rules/rule_update_fixed_regressions.py

78 ↗

(On Diff #220972)

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

lnt/server/ui/views.py

958 ↗

(On Diff #220972)

Is this use of sort something that would be considered within the scope of D67809?

1017 ↗

(On Diff #220972)

This also seems to be used just to iterate through once.

1322 ↗

(On Diff #220972)

Another sorted over a fresh list.

lnt/testing/profile/profilev2impl.py

270 ↗

(On Diff #220972)

keys.extend(f['counters'].keys())?

317 ↗

(On Diff #220972)

I guess the sorted() should go in place of list() where the latter is added a few lines up.

lnt/tests/nt.py

1483 ↗

(On Diff #220972)

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)

1488 ↗

(On Diff #220972)

len(collated_results)

[thopre]

• Remove all wrapping into list that are unecessary due to views being iterable
• Improve uses of views to avoid introducing a call to list() when possible, as per Hubert

[thopre]

examples/run_to_csv.py

10 ↗

(On Diff #220972)

I prefer to apply the change for now and remove it or fix it in a later patch if that's ok. Strange the .next logic didn't detect this case. I've double checked the whole codebase for a call to .next() and didn't find any in python files.

lnt/lnttool/admin.py

50 ↗

(On Diff #220972)

Is there no multithreading going on in LNT?

lnt/server/ui/views.py

958 ↗

(On Diff #220972)

I think the 2to3 fix_idioms only fires if sort is called without parameter. Presumably if the call to sort takes some parameters that might make the folding both lines into sorted too long and thus be difficult to read. I think in this specific case it would indeed be harder to read so I'm enclined to leave it like this.

1497 ↗

(On Diff #220972)

testsuites is only used for iterating below so I removed the list here.

lnt/testing/__init__.py

260 ↗

(On Diff #220972)

I'm removing the list here which I added when committing https://reviews.llvm.org/D65751

lnt/tests/nt.py

1483 ↗

(On Diff #220972)

As mentioned, the 2to3 sort change seems to skip this sort of folding intentionally, presumably to avoid a multiline expression. I'm therefore sticking to having filter and sort as separate steps. I did however changed to use the filter builtin as it makes the intent clearer IMHO.

lnt/util/multidict.py

19–26 ↗

(On Diff #220972)

All uses of these methods already assume this returns views and have been converted to use list if necessary. Ultimately we should remove that file and rely on the pypi package IMHO.

[hubert.reinterpretcast]

examples/run_to_csv.py

10 ↗

(On Diff #220972)

Okay.

lnt/lnttool/admin.py

50 ↗

(On Diff #220972)

config is local to the function, so I don't think it is being concurrently modified here.

lnt/server/ui/views.py

958 ↗

(On Diff #220972)

Okay.

lnt/tests/nt.py

1488 ↗

(On Diff #220972)

I'm not seeing the change. len applied to the dictionary directly works.

[thopre]

Apply len to dictionary rather than dictionary values

lnt/lnttool/admin.py

50 ↗

(On Diff #220972)

I meant in general, I don't know enough the code to make a good judgement call. However IIRW iterating on a views while it is being updated raises an assertion so in the worse case it should be quite obvious what is going on.

lnt/tests/nt.py

1488 ↗

(On Diff #220972)

Oh my bad, missed your point.

[hubert.reinterpretcast]

LGTM.

lnt/trunk/examples/run_to_csv.py

 """Convert the JSON run file passed on stdin to a csv"""
 from __future__ import print_function
 import json
 
 import sys
 
 
 data = json.load(sys.stdin)
 
-titles = data['tests'].itervalues().next().keys()
+titles = list(next(iter(data['tests'].values())).keys())
 titles.insert(0, titles.pop(titles.index("name")))
 
 print(", ".join(titles))
 
 for i, result in data['tests'].items():
 
     for title in titles:
         print(result[title], end=' ')
         print(", ", end=' ')
     print()

lnt/trunk/lnt/server/config.py

def get_database(self, name):
         db_entry = self.databases.get(name)
         if db_entry is None:
             return None
 
         return lnt.server.db.v4db.V4DB(db_entry.path, self,
                                        db_entry.baseline_revision)
 
     def get_database_names(self):
-        return self.databases.keys()
+        return list(self.databases.keys())

lnt/trunk/lnt/server/reporting/summaryreport.py

def _build_data_table(self):
                 # Preload the run parameters.
                 run_parameters_map = dict((r.id, r.parameters)
                                           for r in ts_runs)
 
                 # Load all the samples for all runs we are interested in.
                 columns = [ts.Sample.run_id, ts.Sample.test_id]
                 columns.extend(f.column for f in ts.sample_fields)
                 samples = session.query(*columns).filter(
-                    ts.Sample.run_id.in_(run_id_map.keys()))
+                    ts.Sample.run_id.in_(list(run_id_map.keys())))
                 for sample in samples:
                     run = run_id_map[sample[0]]
                     datapoints = list()
                     for key, value in get_datapoints_for_sample(ts, sample):
                         items = self.data_table.get(key)
                         if items is None:
                             items = [[]
                                      for _ in self.report_orders]

lnt/trunk/lnt/server/ui/regression_views.py

reg_inds = session.query(ts.RegressionIndicator) \
         .all()
     return reg_inds, regressions
 
 
 class EditRegressionForm(Form):
     title = StringField(u'Title', validators=[DataRequired()])
     bug = StringField(u'Bug', validators=[DataRequired()])
     field_changes = MultiCheckboxField("Changes", coerce=int)
-    choices = RegressionState.names.items()
+    choices = list(RegressionState.names.items())
     state = SelectField(u'State', choices=choices)
     edit_state = HiddenField(u'EditState', validators=[DataRequired()])
 
 
 def name(cls):
     """Get a nice name for this object."""
     return cls.__class__.__name__
 

lnt/trunk/lnt/server/ui/views.py

for i, (machine, test, field, field_index) in enumerate(graph_parameters):
 
         # Unless all samples requested, filter out failing tests.
         if not show_failures:
             if field.status_field:
                 q = q.filter((field.status_field.column == PASS) |
                              (field.status_field.column.is_(None)))
 
         # Aggregate by revision.
-        data = multidict.multidict((rev, (val, date, run_id))
-                                   for val, rev, date, run_id in q).items()
+        data = list(multidict.multidict((rev, (val, date, run_id))
+                                        for val, rev, date, run_id in q)
+                    .items())
 
         data.sort(key=lambda sample: convert_revision(sample[0], cache=revision_cache))
 
         graph_datum.append((test.name, data, col, field, url, machine))
 
         # Get baselines for this line
         num_baselines = len(baseline_parameters)
         for baseline_id, (baseline, baseline_title) in \

lnt/trunk/lnt/testing/__init__.py

class Test:
     have the same test name but different info paramaters.
     """
 
     def __init__(self, name, samples, info={}, report_version=2):
         self.name = name
         self.samples = samples
         self.info = dict()
         # Convert keys/values that are not json encodable to strings.
-        for key, value in list(info.items()):
+        for key, value in info.items():
             key = str(key)
             value = str(value)
             self.info[key] = value
         self.report_version = report_version
         self.check()
 
     def check(self):
         """Check object members are adequate to generate an LNT json

lnt/trunk/lnt/testing/profile/profilev2impl.py

def deserialize(self, fobj):
             self.idx_to_name[i] = readString(fobj)
         self.name_to_idx = {v: k
                             for k, v
                             in self.idx_to_name.items()}
 
     def upgrade(self, impl):
         self.idx_to_name = {}
 
-        keys = impl.getTopLevelCounters().keys()
+        keys = list(impl.getTopLevelCounters().keys())
         for f in impl.getFunctions().values():
-            keys += f['counters'].keys()
+            keys.extend(f['counters'].keys())
         keys = sorted(set(keys))
 
         self.idx_to_name = {k: v for k, v in enumerate(keys)}
         self.name_to_idx = {v: k for k, v in enumerate(keys)}
 
 
 class TopLevelCounters(Section):
     def __init__(self, counter_name_pool):
class LineCounters(CompressedSection):
 
     def serialize(self, fobj):
         assert self.impl
 
         self.function_offsets = {}
         start = fobj.tell()
         for fname, f in sorted(self.impl.getFunctions().items()):
             self.function_offsets[fname] = fobj.tell() - start
-            all_counters = f['counters'].keys()
+            all_counters = sorted(f['counters'].keys())
             for counters, address, text in self.impl.getCodeForFunction(fname):
-                for k in sorted(all_counters):
+                for k in all_counters:
                     writeFloat(fobj, counters.get(k, 0))
 
     def deserialize(self, fobj):
         self.data = fobj.read()
 
     def upgrade(self, impl):
         self.impl = impl
         self.function_offsets = {}
class Functions(Section):
 
     def upgrade(self, impl):
         self.impl = impl
         self.functions = self.impl.getFunctions()
 
     def getCodeForFunction(self, fname):
         f = self.functions[fname]
         counter_gen = self.line_counters \
-            .extractForFunction(fname, f['counters'].keys())
+            .extractForFunction(fname, list(f['counters'].keys()))
         address_gen = self.line_addresses.extractForFunction(fname)
         text_gen = self.line_text.extractForFunction(fname)
         for n in xrange(f['length']):
             yield (next(counter_gen), next(address_gen), next(text_gen))
 
     def copy(self, counter_name_pool, line_counters,
              line_addresses, line_text):
         new = copy.copy(self)

lnt/trunk/lnt/tests/nt.py

for full_name, results_status, perf_status in server_results:
         else:
             assert False, "Unexpected server result type." + test_type
         collated_results[test_name] = new_entry
 
     # Double check that all values are there for all tests.
     for test in collated_results.values():
         test.check()
 
-    rerunable_benches = [x for x in collated_results.values()
-                         if x.is_rerunable()]
+    rerunable_benches = list(filter(lambda bench: bench.is_rerunable(),
+                                    collated_results.values()))
     rerunable_benches.sort(key=lambda x: x.name)
     # Now lets do the reruns.
     rerun_results = []
     summary = "Rerunning {} of {} benchmarks."
     logger.info(summary.format(len(rerunable_benches),
-                               len(collated_results.values())))
+                               len(collated_results)))
 
     for i, bench in enumerate(rerunable_benches):
         logger.info("Rerunning: {} [{}/{}]".format(bench.name,
                                                    i + 1,
                                                    len(rerunable_benches)))
 
         fresh_samples, t_no_errors = rerun_test(config,
                                                 bench.name, NUMBER_OF_RERUNS)

lnt/trunk/tests/server/ui/V4Pages.py

def dump_html(html_string):
     for linenr, line in enumerate(html_string.split('\n')):
         print("%4d:%s" % (linenr + 1, line))
 
 
 def get_xml_tree(html_string):
     try:
         parser = ET.XMLParser()
         parser.parser.UseForeignDTD(True)
-        parser.entity.update((x, unichr(i)) for x, i in name2codepoint.iteritems())
+        parser.entity.update((x, unichr(i)) for x, i in name2codepoint.items())
         tree = ET.fromstring(html_string, parser=parser)
     except:  # noqa FIXME: figure out what we expect this to throw.
         dump_html(html_string)
         raise
     return tree
 
 
 def find_table_by_thead_content(tree, table_head_contents):

lnt/trunk/tests/server/ui/test_roundtrip.py

def _compare_results(self, after_submit_run, before_submit_run):
         an_id = 1234567
         before_submit_run['run']['id'] = an_id
         before_submit_run['machine']['id'] = an_id
         after_submit_run['run']['id'] = an_id
         after_submit_run['machine']['id'] = an_id
         before_submit_run['run']['order_id'] = an_id
         after_submit_run['run']['order_id'] = an_id
 
-        self.assertEqual(before_submit_run.keys(), after_submit_run.keys())
+        self.assertEqual(list(before_submit_run.keys()), list(after_submit_run.keys()))
         # Machine and run will be dicts, compare them directly.
         for k in ['machine', 'run']:
             self.assertEqual(before_submit_run[k], after_submit_run[k])
         # The order of the tests might have changed, so sort before they are compared.
         before_submit_tests = sorted(before_submit_run['tests'])
         after_submit_tests = sorted(after_submit_run['tests'])
         for i, _ in enumerate(before_submit_tests):
             before_submit_tests[i]['run_id'] = 1234