From 477f8cf7de57fe47a2fd03b1a232386dcd22a333 Mon Sep 17 00:00:00 2001
From: Deepak Cherian <dcherian@users.noreply.github.com>
Date: Sat, 4 Nov 2023 14:25:35 -0600
Subject: [PATCH] Better attr diff for `testing.assert_identical` (#8400)

* [WIP] Better attr diff

* Fix test

* Update xarray/core/formatting.py

Co-authored-by: Justus Magin <keewis@users.noreply.github.com>

* Improvements

---------

Co-authored-by: Justus Magin <keewis@users.noreply.github.com>
---
 xarray/core/formatting.py       | 30 +++++++++++++++++++++++++++---
 xarray/tests/test_formatting.py | 26 +++++++++++++++++++-------
 2 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/xarray/core/formatting.py b/xarray/core/formatting.py
index 561b8d3cc0d..a915e9acbf3 100644
--- a/xarray/core/formatting.py
+++ b/xarray/core/formatting.py
@@ -783,9 +783,11 @@ def extra_items_repr(extra_keys, mapping, ab_side, kwargs):
         try:
             # compare xarray variable
             if not callable(compat):
-                compatible = getattr(a_mapping[k], compat)(b_mapping[k])
+                compatible = getattr(a_mapping[k].variable, compat)(
+                    b_mapping[k].variable
+                )
             else:
-                compatible = compat(a_mapping[k], b_mapping[k])
+                compatible = compat(a_mapping[k].variable, b_mapping[k].variable)
             is_variable = True
         except AttributeError:
             # compare attribute value
@@ -804,11 +806,21 @@ def extra_items_repr(extra_keys, mapping, ab_side, kwargs):
 
             if compat == "identical" and is_variable:
                 attrs_summary = []
+                a_attrs = a_mapping[k].attrs
+                b_attrs = b_mapping[k].attrs
 
+                attrs_to_print = set(a_attrs) ^ set(b_attrs)
+                attrs_to_print.update(
+                    {k for k in set(a_attrs) & set(b_attrs) if a_attrs[k] != b_attrs[k]}
+                )
                 for m in (a_mapping, b_mapping):
                     attr_s = "\n".join(
-                        summarize_attr(ak, av) for ak, av in m[k].attrs.items()
+                        "    " + summarize_attr(ak, av)
+                        for ak, av in m[k].attrs.items()
+                        if ak in attrs_to_print
                     )
+                    if attr_s:
+                        attr_s = "    Differing variable attributes:\n" + attr_s
                     attrs_summary.append(attr_s)
 
                 temp = [
@@ -816,6 +828,18 @@ def extra_items_repr(extra_keys, mapping, ab_side, kwargs):
                     for var_s, attr_s in zip(temp, attrs_summary)
                 ]
 
+                # TODO: It should be possible recursively use _diff_mapping_repr
+                #       instead of explicitly handling variable attrs specially.
+                #       That would require some refactoring.
+                # newdiff = _diff_mapping_repr(
+                #     {k: v for k,v in a_attrs.items() if k in attrs_to_print},
+                #     {k: v for k,v in b_attrs.items() if k in attrs_to_print},
+                #     compat=compat,
+                #     summarizer=summarize_attr,
+                #     title="Variable Attributes"
+                # )
+                # temp += [newdiff]
+
             diff_items += [ab_side + s[1:] for ab_side, s in zip(("L", "R"), temp)]
 
     if diff_items:
diff --git a/xarray/tests/test_formatting.py b/xarray/tests/test_formatting.py
index d5c8e0c0d0a..96bb9c8a3a7 100644
--- a/xarray/tests/test_formatting.py
+++ b/xarray/tests/test_formatting.py
@@ -406,19 +406,27 @@ def test_diff_dataset_repr(self) -> None:
                 "var2": ("x", np.array([3, 4], dtype="int64")),
             },
             coords={
-                "x": np.array(["a", "b"], dtype="U1"),
+                "x": (
+                    "x",
+                    np.array(["a", "b"], dtype="U1"),
+                    {"foo": "bar", "same": "same"},
+                ),
                 "y": np.array([1, 2, 3], dtype="int64"),
             },
-            attrs={"units": "m", "description": "desc"},
+            attrs={"title": "mytitle", "description": "desc"},
         )
 
         ds_b = xr.Dataset(
             data_vars={"var1": ("x", np.array([1, 2], dtype="int64"))},
             coords={
-                "x": ("x", np.array(["a", "c"], dtype="U1"), {"source": 0}),
+                "x": (
+                    "x",
+                    np.array(["a", "c"], dtype="U1"),
+                    {"source": 0, "foo": "baz", "same": "same"},
+                ),
                 "label": ("x", np.array([1, 2], dtype="int64")),
             },
-            attrs={"units": "kg"},
+            attrs={"title": "newtitle"},
         )
 
         byteorder = "<" if sys.byteorder == "little" else ">"
@@ -429,8 +437,12 @@ def test_diff_dataset_repr(self) -> None:
             (x: 2, y: 3) != (x: 2)
         Differing coordinates:
         L * x        (x) %cU1 'a' 'b'
+            Differing variable attributes:
+                foo: bar
         R * x        (x) %cU1 'a' 'c'
-            source: 0
+            Differing variable attributes:
+                source: 0
+                foo: baz
         Coordinates only on the left object:
           * y        (y) int64 1 2 3
         Coordinates only on the right object:
@@ -441,8 +453,8 @@ def test_diff_dataset_repr(self) -> None:
         Data variables only on the left object:
             var2     (x) int64 3 4
         Differing attributes:
-        L   units: m
-        R   units: kg
+        L   title: mytitle
+        R   title: newtitle
         Attributes only on the left object:
             description: desc"""
             % (byteorder, byteorder)