rebaseline_server: add pixel diffs, and sorting by diff metrics

gm/rebaseline_server/results.py

 #!/usr/bin/python
 
 """
 Copyright 2013 Google Inc.
 
 Use of this source code is governed by a BSD-style license that can be
 found in the LICENSE file.
 
 Repackage expected/actual GM results as needed by our HTML rebaseline viewer.
 """
@@ skipping 11 matching lines @@
 #
 # We need to add the 'gm' directory, so that we can import gm_json.py within
 # that directory.  That script allows us to parse the actual-results.json file
 # written out by the GM tool.
 # Make sure that the 'gm' dir is in the PYTHONPATH, but add it at the *end*
 # so any dirs that are already in the PYTHONPATH will be preferred.
 GM_DIRECTORY = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
 if GM_DIRECTORY not in sys.path:
   sys.path.append(GM_DIRECTORY)
 import gm_json
+import imagediffdb
 
 IMAGE_FILENAME_RE = re.compile(gm_json.IMAGE_FILENAME_PATTERN)
 IMAGE_FILENAME_FORMATTER = '%s_%s.png'  # pass in (testname, config)
 
 FIELDS_PASSED_THRU_VERBATIM = [
     gm_json.JSONKEY_EXPECTEDRESULTS_BUGS,
     gm_json.JSONKEY_EXPECTEDRESULTS_IGNOREFAILURE,
     gm_json.JSONKEY_EXPECTEDRESULTS_REVIEWED,
 ]
 CATEGORIES_TO_SUMMARIZE = [
     'builder', 'test', 'config', 'resultType',
     gm_json.JSONKEY_EXPECTEDRESULTS_IGNOREFAILURE,
     gm_json.JSONKEY_EXPECTEDRESULTS_REVIEWED,
 ]
 
 RESULTS_ALL = 'all'
 RESULTS_FAILURES = 'failures'
 
 class Results(object):
   """ Loads actual and expected results from all builders, supplying combined
   reports as requested.
 
   Once this object has been constructed, the results (in self._results[])
   are immutable.  If you want to update the results based on updated JSON
   file contents, you will need to create a new Results object."""
 
-  def __init__(self, actuals_root, expected_root):
+  def __init__(self, actuals_root, expected_root, generated_images_root):
     """
     Args:
       actuals_root: root directory containing all actual-results.json files
       expected_root: root directory containing all expected-results.json files
+      generated_images_root: directory within which to create all pixels diffs;
+          if this directory does not yet exist, it will be created
     """
+    self._image_diff_db = imagediffdb.ImageDiffDB(generated_images_root)
     self._actuals_root = actuals_root
     self._expected_root = expected_root
     self._load_actual_and_expected()
     self._timestamp = int(time.time())
 
   def get_timestamp(self):
     """Return the time at which this object was created, in seconds past epoch
     (UTC).
     """
     return self._timestamp
@@ skipping 168 matching lines @@
     # Check: did we write out the set of per-builder dictionaries we
     # expected to?
     expected_builders_written = sorted(meta_dict.keys())
     actual_builders_written.sort()
     if expected_builders_written != actual_builders_written:
       raise KeyError(
           'expected to write dicts for builders %s, but actually wrote them '
           'for builders %s' % (
               expected_builders_written, actual_builders_written))
 
+  def _generate_pixel_diffs_if_needed(self, test, expected_image, actual_image):
+    """If expected_image and actual_image both exist but are different,
+    add the image pair to self._image_diff_db and generate pixel diffs.
+
+    Args:
+      test: string; name of test
+      expected_image: (hashType, hashDigest) tuple describing the expected image
+      actual_image: (hashType, hashDigest) tuple describing the actual image
+    """
+    if expected_image == actual_image:
+      return
+
+    (expected_hashtype, expected_hashdigest) = expected_image
+    (actual_hashtype, actual_hashdigest) = actual_image
+    if ((not expected_hashtype) or (not expected_hashdigest) or

[jcgregorio, 2013/11/06 18:47:28]

are you checking for None's?

if None in [expected_hashtype, expected_hashdigest, actual_hashtype,
actual_hashdigest]:

might be clearer.

[epoger, 2013/11/07 21:11:53]

Done.

+        (not actual_hashtype) or (not actual_hashdigest)):
+      return
+
+    expected_url = gm_json.CreateGmActualUrl(
+        test_name=test, hash_type=expected_hashtype,
+        hash_digest=expected_hashdigest)
+    actual_url = gm_json.CreateGmActualUrl(
+        test_name=test, hash_type=actual_hashtype,
+        hash_digest=actual_hashdigest)
+    self._image_diff_db.add_image_pair(
+        expected_image_locator=expected_hashdigest,
+        expected_image_url=expected_url,
+        actual_image_locator=actual_hashdigest,
+        actual_image_url=actual_url)
+
   def _load_actual_and_expected(self):
     """Loads the results of all tests, across all builders (based on the
     files within self._actuals_root and self._expected_root),
     and stores them in self._results.
     """
     actual_builder_dicts = Results._read_dicts_from_root(self._actuals_root)
     expected_builder_dicts = Results._read_dicts_from_root(self._expected_root)
 
     categories_all = {}
     categories_failures = {}
@@ skipping 74 matching lines @@
           # category?  Maybe we should not rely on the result_type
           # categories recorded within the gm_actuals AT ALL, and
           # instead evaluate the result_type ourselves based on what
           # we see in expectations vs actual checksum?
           if expected_image == actual_image:
             updated_result_type = gm_json.JSONKEY_ACTUALRESULTS_SUCCEEDED
           else:
             updated_result_type = result_type
 
           (test, config) = IMAGE_FILENAME_RE.match(image_name).groups()
+          self._generate_pixel_diffs_if_needed(
+              test=test, expected_image=expected_image,

[jcgregorio, 2013/11/06 18:47:28]

self._generate_diffs_if_needed(test, expected_image, actual_image)

[epoger, 2013/11/07 21:11:53]

I've become a fan of named arguments (I like that if someone changes/reorders
the function arguments later, the callsites don't have to change).

The extra verbosity is disappointing, but in Python's world where there is no
compile-time checking, these safeguards help me sleep easier.

Is there a way we can better balance those two conflicting desires?

[jcgregorio, 2013/11/08 02:02:29]

NP, if you're willing to do the typing :-)

On 2013/11/07 21:11:53, epoger wrote:

+              actual_image=actual_image)
           results_for_this_test = {
               'resultType': updated_result_type,
               'builder': builder,
               'test': test,
               'config': config,
               'actualHashType': actual_image[0],
               'actualHashDigest': str(actual_image[1]),
               'expectedHashType': expected_image[0],
               'expectedHashDigest': str(expected_image[1]),
 
               # FIELDS_PASSED_THRU_VERBATIM that may be overwritten below...
               gm_json.JSONKEY_EXPECTEDRESULTS_IGNOREFAILURE: False,
           }
           if expectations_per_test:
             for field in FIELDS_PASSED_THRU_VERBATIM:
               results_for_this_test[field] = expectations_per_test.get(field)
+
+          try:
+            diff_record = self._image_diff_db.get_diff_record(
+                expected_image_locator=expected_image[1],
+                actual_image_locator=actual_image[1])
+            results_for_this_test['percentDifferingPixels'] = (
+                diff_record.get_percent_pixels_differing())
+            results_for_this_test['weightedDiffMeasure'] = (
+                diff_record.get_weighted_diff_measure())
+          except KeyError:

[jcgregorio, 2013/11/06 18:47:28]

log the error?

[epoger, 2013/11/07 21:11:53]

Done.

+            pass
+
           Results._add_to_category_dict(categories_all, results_for_this_test)
           data_all.append(results_for_this_test)
 
           # TODO(epoger): In effect, we have a list of resultTypes that we
           # include in the different result lists (data_all and data_failures).
           # This same list should be used by the calls to
           # Results._ensure_included_in_category_dict() earlier on.
           if updated_result_type != gm_json.JSONKEY_ACTUALRESULTS_SUCCEEDED:
             Results._add_to_category_dict(categories_failures,
                                           results_for_this_test)
@@ skipping 41 matching lines @@
       category_dict: category dict-of-dicts to modify
       category_name: category name, as a string
       category_values: list of values we want to make sure are represented
                        for this category
     """
     if not category_dict.get(category_name):
       category_dict[category_name] = {}
     for category_value in category_values:
       if not category_dict[category_name].get(category_value):
         category_dict[category_name][category_value] = 0