also apply combine_attrs to the attrs of the variables

doc/whats-new.rst

@@ -22,7 +22,8 @@ v0.17.1 (unreleased)
 
 New Features
 ~~~~~~~~~~~~
-
+- apply ``combine_attrs`` recursively (:pull:`4902`).
+  By `Justus Magin <https://github.com/keewis>`_.
 
 Breaking changes
 ~~~~~~~~~~~~~~~~

xarray/core/concat.py

@@ -508,7 +508,7 @@ def ensure_common_dims(vars):
                 vars = ensure_common_dims([ds[k].variable for ds in datasets])
             except KeyError:
                 raise ValueError("%r is not present in all datasets." % k)
-            combined = concat_vars(vars, dim, positions)
+            combined = concat_vars(vars, dim, positions, combine_attrs=combine_attrs)
             assert isinstance(combined, Variable)
             result_vars[k] = combined
         elif k in result_vars:
@@ -572,7 +572,7 @@ def _dataarray_concat(
         positions,
         fill_value=fill_value,
         join=join,
-        combine_attrs="drop",
+        combine_attrs=combine_attrs,
     )
 
     merged_attrs = merge_attrs([da.attrs for da in arrays], combine_attrs)

xarray/core/merge.py

@@ -164,6 +164,7 @@ def merge_collected(
     grouped: Dict[Hashable, List[MergeElement]],
     prioritized: Mapping[Hashable, MergeElement] = None,
     compat: str = "minimal",
+    combine_attrs="override",
 ) -> Tuple[Dict[Hashable, Variable], Dict[Hashable, pd.Index]]:
     """Merge dicts of variables, while resolving conflicts appropriately.
 
@@ -227,6 +228,9 @@ def merge_collected(
                 variables = [variable for variable, _ in elements_list]
                 try:
                     merged_vars[name] = unique_variable(name, variables, compat)
+                    merged_vars[name].attrs = merge_attrs(
+                        [var.attrs for var in variables], combine_attrs=combine_attrs
+                    )
                 except MergeError:
                     if compat != "minimal":
                         # we need more than "minimal" compatibility (for which
@@ -613,7 +617,9 @@ def merge_core(
     collected = collect_variables_and_indexes(aligned)
 
     prioritized = _get_priority_vars_and_indexes(aligned, priority_arg, compat=compat)
-    variables, out_indexes = merge_collected(collected, prioritized, compat=compat)
+    variables, out_indexes = merge_collected(
+        collected, prioritized, compat=compat, combine_attrs=combine_attrs
+    )
     assert_unique_multiindex_level_names(variables)
 
     dims = calculate_dimensions(variables)

xarray/core/variable.py

@@ -1769,7 +1769,14 @@ def reduce(
         return Variable(dims, data, attrs=attrs)
 
     @classmethod
-    def concat(cls, variables, dim="concat_dim", positions=None, shortcut=False):
+    def concat(
+        cls,
+        variables,
+        dim="concat_dim",
+        positions=None,
+        shortcut=False,
+        combine_attrs="override",
+    ):
         """Concatenate variables along a new or existing dimension.
 
         Parameters
@@ -1792,13 +1799,27 @@ def concat(cls, variables, dim="concat_dim", positions=None, shortcut=False):
             This option is used internally to speed-up groupby operations.
             If `shortcut` is True, some checks of internal consistency between
             arrays to concatenate are skipped.
+        combine_attrs : {"drop", "identical", "no_conflicts", "drop_conflicts", \
+                         "override"}, default: "override"
+            String indicating how to combine attrs of the objects being merged:
+
+            - "drop": empty attrs on returned Dataset.
+            - "identical": all attrs must be the same on every object.
+            - "no_conflicts": attrs from all objects are combined, any that have
+              the same name must also have the same value.
+            - "drop_conflicts": attrs from all objects are combined, any that have
+              the same name but different values are dropped.
+            - "override": skip comparing and copy attrs from the first dataset to
+              the result.
 
         Returns
         -------
         stacked : Variable
             Concatenated Variable formed by stacking all the supplied variables
             along the given dimension.
         """
+        from .merge import merge_attrs
+
         if not isinstance(dim, str):
             (dim,) = dim.dims
 
@@ -1823,7 +1844,9 @@ def concat(cls, variables, dim="concat_dim", positions=None, shortcut=False):
             dims = (dim,) + first_var.dims
             data = duck_array_ops.stack(arrays, axis=axis)
 
-        attrs = dict(first_var.attrs)
+        attrs = merge_attrs(
+            [var.attrs for var in variables], combine_attrs=combine_attrs
+        )
         encoding = dict(first_var.encoding)
         if not shortcut:
             for var in variables:
@@ -2565,12 +2588,21 @@ def __setitem__(self, key, value):
         raise TypeError("%s values cannot be modified" % type(self).__name__)
 
     @classmethod
-    def concat(cls, variables, dim="concat_dim", positions=None, shortcut=False):
+    def concat(
+        cls,
+        variables,
+        dim="concat_dim",
+        positions=None,
+        shortcut=False,
+        combine_attrs="override",
+    ):
         """Specialized version of Variable.concat for IndexVariable objects.
 
         This exists because we want to avoid converting Index objects to NumPy
         arrays, if possible.
         """
+        from .merge import merge_attrs
+
         if not isinstance(dim, str):
             (dim,) = dim.dims
 
@@ -2597,12 +2629,13 @@ def concat(cls, variables, dim="concat_dim", positions=None, shortcut=False):
         # keep as str if possible as pandas.Index uses object (converts to numpy array)
         data = maybe_coerce_to_str(data, variables)
 
-        attrs = dict(first_var.attrs)
+        attrs = merge_attrs(
+            [var.attrs for var in variables], combine_attrs=combine_attrs
+        )
         if not shortcut:
             for var in variables:
                 if var.dims != first_var.dims:
                     raise ValueError("inconsistent dimensions")
-                utils.remove_incompatible_items(attrs, var.attrs)
 
         return cls(first_var.dims, data, attrs)
 
@@ -2776,7 +2809,13 @@ def _broadcast_compat_data(self, other):
     return self_data, other_data, dims
 
 
-def concat(variables, dim="concat_dim", positions=None, shortcut=False):
+def concat(
+    variables,
+    dim="concat_dim",
+    positions=None,
+    shortcut=False,
+    combine_attrs="override",
+):
     """Concatenate variables along a new or existing dimension.
 
     Parameters
@@ -2799,6 +2838,18 @@ def concat(variables, dim="concat_dim", positions=None, shortcut=False):
         This option is used internally to speed-up groupby operations.
         If `shortcut` is True, some checks of internal consistency between
         arrays to concatenate are skipped.
+    combine_attrs : {"drop", "identical", "no_conflicts", "drop_conflicts", \
+                     "override"}, default: "override"
+        String indicating how to combine attrs of the objects being merged:
+
+        - "drop": empty attrs on returned Dataset.
+        - "identical": all attrs must be the same on every object.
+        - "no_conflicts": attrs from all objects are combined, any that have
+          the same name must also have the same value.
+        - "drop_conflicts": attrs from all objects are combined, any that have
+          the same name but different values are dropped.
+        - "override": skip comparing and copy attrs from the first dataset to
+          the result.
 
     Returns
     -------
@@ -2808,9 +2859,9 @@ def concat(variables, dim="concat_dim", positions=None, shortcut=False):
     """
     variables = list(variables)
     if all(isinstance(v, IndexVariable) for v in variables):
-        return IndexVariable.concat(variables, dim, positions, shortcut)
+        return IndexVariable.concat(variables, dim, positions, shortcut, combine_attrs)
     else:
-        return Variable.concat(variables, dim, positions, shortcut)
+        return Variable.concat(variables, dim, positions, shortcut, combine_attrs)
 
 
 def assert_unique_multiindex_level_names(variables):

xarray/tests/test_combine.py

@@ -4,7 +4,14 @@
 import numpy as np
 import pytest
 
-from xarray import DataArray, Dataset, combine_by_coords, combine_nested, concat
+from xarray import (
+    DataArray,
+    Dataset,
+    MergeError,
+    combine_by_coords,
+    combine_nested,
+    concat,
+)
 from xarray.core import dtypes
 from xarray.core.combine import (
     _check_shape_tile_ids,
@@ -19,6 +26,13 @@
 from .test_dataset import create_test_data
 
 
+def clear_attrs(ds):
+    for var in ds.variables.values():
+        var.attrs.clear()
+    ds.attrs.clear()
+    return ds
+
+
 def assert_combined_tile_ids_equal(dict1, dict2):
     assert len(dict1) == len(dict2)
     for k, v in dict1.items():
@@ -471,7 +485,8 @@ def test_concat_name_symmetry(self):
         assert_identical(x_first, y_first)
 
     def test_concat_one_dim_merge_another(self):
-        data = create_test_data()
+        data = clear_attrs(create_test_data())
+
         data1 = data.copy(deep=True)
         data2 = data.copy(deep=True)
 
@@ -497,7 +512,7 @@ def test_auto_combine_2d(self):
         assert_equal(result, expected)
 
     def test_auto_combine_2d_combine_attrs_kwarg(self):
-        ds = create_test_data
+        ds = lambda x: clear_attrs(create_test_data(x))
 
         partway1 = concat([ds(0), ds(3)], dim="dim1")
         partway2 = concat([ds(1), ds(4)], dim="dim1")
@@ -743,6 +758,142 @@ def test_combine_nested_combine_attrs_drop_conflicts(self):
         )
         assert_identical(expected, actual)
 
+    @pytest.mark.parametrize(
+        "combine_attrs, attrs1, attrs2, expected_attrs, expect_exception",
+        [
+            (
+                "no_conflicts",
+                {"a": 1, "b": 2},
+                {"a": 1, "c": 3},
+                {"a": 1, "b": 2, "c": 3},
+                False,
+            ),
+            ("no_conflicts", {"a": 1, "b": 2}, {}, {"a": 1, "b": 2}, False),
+            ("no_conflicts", {}, {"a": 1, "c": 3}, {"a": 1, "c": 3}, False),
+            (
+                "no_conflicts",
+                {"a": 1, "b": 2},
+                {"a": 4, "c": 3},
+                {"a": 1, "b": 2, "c": 3},
+                True,
+            ),
+            ("drop", {"a": 1, "b": 2}, {"a": 1, "c": 3}, {}, False),
+            ("identical", {"a": 1, "b": 2}, {"a": 1, "b": 2}, {"a": 1, "b": 2}, False),
+            ("identical", {"a": 1, "b": 2}, {"a": 1, "c": 3}, {"a": 1, "b": 2}, True),
+            (
+                "override",
+                {"a": 1, "b": 2},
+                {"a": 4, "b": 5, "c": 3},
+                {"a": 1, "b": 2},
+                False,
+            ),
+            (
+                "drop_conflicts",
+                {"a": 1, "b": 2, "c": 3},
+                {"b": 1, "c": 3, "d": 4},
+                {"a": 1, "c": 3, "d": 4},
+                False,
+            ),
+        ],
+    )
+    def test_combine_nested_combine_attrs_variables(
+        self, combine_attrs, attrs1, attrs2, expected_attrs, expect_exception
+    ):
+        """check that combine_attrs is used on data variables and coords"""
+        data1 = Dataset(
+            {
+                "a": ("x", [1, 2], attrs1),
+                "b": ("x", [3, -1], attrs1),
+                "x": ("x", [0, 1], attrs1),
+            }
+        )
+        data2 = Dataset(
+            {
+                "a": ("x", [2, 3], attrs2),
+                "b": ("x", [-2, 1], attrs2),
+                "x": ("x", [2, 3], attrs2),
+            }
+        )
+
+        if expect_exception:
+            with raises_regex(MergeError, "combine_attrs"):
+                combine_by_coords([data1, data2], combine_attrs=combine_attrs)
+        else:
+            actual = combine_by_coords([data1, data2], combine_attrs=combine_attrs)
+            expected = Dataset(
+                {
+                    "a": ("x", [1, 2, 2, 3], expected_attrs),
+                    "b": ("x", [3, -1, -2, 1], expected_attrs),
+                },
+                {"x": ("x", [0, 1, 2, 3], expected_attrs)},
+            )
+
+            assert_identical(actual, expected)
+
+    @pytest.mark.parametrize(
+        "combine_attrs, attrs1, attrs2, expected_attrs, expect_exception",
+        [
+            (
+                "no_conflicts",
+                {"a": 1, "b": 2},
+                {"a": 1, "c": 3},
+                {"a": 1, "b": 2, "c": 3},
+                False,
+            ),
+            ("no_conflicts", {"a": 1, "b": 2}, {}, {"a": 1, "b": 2}, False),
+            ("no_conflicts", {}, {"a": 1, "c": 3}, {"a": 1, "c": 3}, False),
+            (
+                "no_conflicts",
+                {"a": 1, "b": 2},
+                {"a": 4, "c": 3},
+                {"a": 1, "b": 2, "c": 3},
+                True,
+            ),
+            ("drop", {"a": 1, "b": 2}, {"a": 1, "c": 3}, {}, False),
+            ("identical", {"a": 1, "b": 2}, {"a": 1, "b": 2}, {"a": 1, "b": 2}, False),
+            ("identical", {"a": 1, "b": 2}, {"a": 1, "c": 3}, {"a": 1, "b": 2}, True),
+            (
+                "override",
+                {"a": 1, "b": 2},
+                {"a": 4, "b": 5, "c": 3},
+                {"a": 1, "b": 2},
+                False,
+            ),
+            (
+                "drop_conflicts",
+                {"a": 1, "b": 2, "c": 3},
+                {"b": 1, "c": 3, "d": 4},
+                {"a": 1, "c": 3, "d": 4},
+                False,
+            ),
+        ],
+    )
+    def test_combine_by_coords_combine_attrs_variables(
+        self, combine_attrs, attrs1, attrs2, expected_attrs, expect_exception
+    ):
+        """check that combine_attrs is used on data variables and coords"""
+        data1 = Dataset(
+            {"x": ("a", [0], attrs1), "y": ("a", [0], attrs1), "a": ("a", [0], attrs1)}
+        )
+        data2 = Dataset(
+            {"x": ("a", [1], attrs2), "y": ("a", [1], attrs2), "a": ("a", [1], attrs2)}
+        )
+
+        if expect_exception:
+            with raises_regex(MergeError, "combine_attrs"):
+                combine_by_coords([data1, data2], combine_attrs=combine_attrs)
+        else:
+            actual = combine_by_coords([data1, data2], combine_attrs=combine_attrs)
+            expected = Dataset(
+                {
+                    "x": ("a", [0, 1], expected_attrs),
+                    "y": ("a", [0, 1], expected_attrs),
+                    "a": ("a", [0, 1], expected_attrs),
+                }
+            )
+
+            assert_identical(actual, expected)
+
     def test_infer_order_from_coords(self):
         data = create_test_data()
         objs = [data.isel(dim2=slice(4, 9)), data.isel(dim2=slice(4))]

xarray/tests/test_concat.py

@@ -313,7 +313,6 @@ def test_concat_combine_attrs_kwarg(
 
             assert_identical(actual, expected)
 
-    @pytest.mark.skip(reason="not implemented, yet (see #4827)")
     @pytest.mark.parametrize(
         "combine_attrs, attrs1, attrs2, expected_attrs, expect_exception",
         [