From d8bb07c8c3d0be90a190baf9fd4522892002bad6 Mon Sep 17 00:00:00 2001 From: dcherian Date: Tue, 1 Oct 2019 12:08:36 -0600 Subject: [PATCH 1/6] Fix concat bug when concatenating unlabeled dimensions. --- xarray/core/concat.py | 2 -- xarray/tests/test_concat.py | 9 +++++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/xarray/core/concat.py b/xarray/core/concat.py index e68c247d880..a076252d28e 100644 --- a/xarray/core/concat.py +++ b/xarray/core/concat.py @@ -177,8 +177,6 @@ def _calc_concat_over(datasets, dim, dim_names, data_vars, coords, compat): if dim not in ds.dims: if dim in ds: ds = ds.set_coords(dim) - else: - raise ValueError("%r is not present in all datasets" % dim) concat_over.update(k for k, v in ds.variables.items() if dim in v.dims) concat_dim_lengths.append(ds.dims.get(dim, 1)) diff --git a/xarray/tests/test_concat.py b/xarray/tests/test_concat.py index 00428f70966..5986a9b7f04 100644 --- a/xarray/tests/test_concat.py +++ b/xarray/tests/test_concat.py @@ -41,9 +41,6 @@ def test_concat_compat(): for var in ["has_x", "no_x_y"]: assert "y" not in result[var] - with raises_regex(ValueError, "'q' is not present in all datasets"): - concat([ds1, ds2], dim="q", data_vars="all", compat="broadcast_equals") - class TestConcatDataset: @pytest.fixture @@ -89,7 +86,11 @@ def test_concat_coords_kwarg(self, data, dim, coords): assert_equal(data["extra"], actual["extra"]) def test_concat(self, data): - split_data = [data.isel(dim1=slice(3)), data.isel(dim1=slice(3, None))] + split_data = [ + data.isel(dim1=slice(3)), + data.isel(dim1=3), + data.isel(dim1=slice(4, None)), + ] assert_identical(data, concat(split_data, "dim1")) def test_concat_dim_precedence(self, data): From bdc63ff5207883e2f4a504e4dcfd72d3e8d50117 Mon Sep 17 00:00:00 2001 From: dcherian Date: Tue, 1 Oct 2019 12:12:54 -0600 Subject: [PATCH 2/6] Add whats-new --- doc/whats-new.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index f9c952f6752..28ae2deb49f 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -27,6 +27,8 @@ Bug fixes - Line plots with the ``x`` or ``y`` argument set to a 1D non-dimensional coord now plot the correct data for 2D DataArrays. (:issue:`3334`). By `Tom Nicholas `_. +- Fix error in concatenating unlabeled dimensions (:pull:`3362`). + By `Deepak Cherian `_. Documentation ~~~~~~~~~~~~~ From db39a0776afdb3f8665f4a15822b74d8aa76af5f Mon Sep 17 00:00:00 2001 From: dcherian Date: Tue, 1 Oct 2019 13:06:58 -0600 Subject: [PATCH 3/6] Add back older test. --- xarray/tests/test_concat.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/xarray/tests/test_concat.py b/xarray/tests/test_concat.py index 5986a9b7f04..9cc9224bc9c 100644 --- a/xarray/tests/test_concat.py +++ b/xarray/tests/test_concat.py @@ -41,6 +41,9 @@ def test_concat_compat(): for var in ["has_x", "no_x_y"]: assert "y" not in result[var] + with raises_regex(ValueError, "One or more of the specified variables"): + concat([ds1, ds2], dim="q") + class TestConcatDataset: @pytest.fixture From c33ca34a012c97c82be278fb0b8c1aeb000a284d Mon Sep 17 00:00:00 2001 From: dcherian Date: Tue, 1 Oct 2019 13:21:24 -0600 Subject: [PATCH 4/6] fix test --- xarray/tests/test_combine.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xarray/tests/test_combine.py b/xarray/tests/test_combine.py index 6037669ac07..379f661917d 100644 --- a/xarray/tests/test_combine.py +++ b/xarray/tests/test_combine.py @@ -756,7 +756,7 @@ def test_auto_combine(self): auto_combine(objs) objs = [Dataset({"x": [0], "y": [0]}), Dataset({"x": [0]})] - with raises_regex(ValueError, "'y' is not present in all datasets"): + with raises_regex(KeyError, "'y'"): auto_combine(objs) def test_auto_combine_previously_failed(self): From abdff2e7c37c4f92c9b706ec41e56671627f8b78 Mon Sep 17 00:00:00 2001 From: dcherian Date: Tue, 1 Oct 2019 17:56:24 -0600 Subject: [PATCH 5/6] Revert "fix test" This reverts commit c33ca34a012c97c82be278fb0b8c1aeb000a284d. --- xarray/tests/test_combine.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xarray/tests/test_combine.py b/xarray/tests/test_combine.py index 379f661917d..6037669ac07 100644 --- a/xarray/tests/test_combine.py +++ b/xarray/tests/test_combine.py @@ -756,7 +756,7 @@ def test_auto_combine(self): auto_combine(objs) objs = [Dataset({"x": [0], "y": [0]}), Dataset({"x": [0]})] - with raises_regex(KeyError, "'y'"): + with raises_regex(ValueError, "'y' is not present in all datasets"): auto_combine(objs) def test_auto_combine_previously_failed(self): From a0a19def23209191f4b34356a01beeea1c56a316 Mon Sep 17 00:00:00 2001 From: dcherian Date: Tue, 1 Oct 2019 17:58:40 -0600 Subject: [PATCH 6/6] better fix --- xarray/core/concat.py | 11 ++++++++++- xarray/tests/test_concat.py | 4 +++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/xarray/core/concat.py b/xarray/core/concat.py index a076252d28e..d7da3153b76 100644 --- a/xarray/core/concat.py +++ b/xarray/core/concat.py @@ -360,12 +360,21 @@ def ensure_common_dims(vars): # n.b. this loop preserves variable order, needed for groupby. for k in datasets[0].variables: if k in concat_over: - vars = ensure_common_dims([ds.variables[k] for ds in datasets]) + try: + vars = ensure_common_dims([ds.variables[k] for ds in datasets]) + except KeyError: + raise ValueError("%r is not present in all datasets." % k) combined = concat_vars(vars, dim, positions) assert isinstance(combined, Variable) result_vars[k] = combined result = Dataset(result_vars, attrs=result_attrs) + absent_coord_names = coord_names - set(result.variables) + if absent_coord_names: + raise ValueError( + "Variables %r are coordinates in some datasets but not others." + % absent_coord_names + ) result = result.set_coords(coord_names) result.encoding = result_encoding diff --git a/xarray/tests/test_concat.py b/xarray/tests/test_concat.py index 9cc9224bc9c..8ef5eb5a541 100644 --- a/xarray/tests/test_concat.py +++ b/xarray/tests/test_concat.py @@ -41,8 +41,10 @@ def test_concat_compat(): for var in ["has_x", "no_x_y"]: assert "y" not in result[var] - with raises_regex(ValueError, "One or more of the specified variables"): + with raises_regex(ValueError, "coordinates in some datasets but not others"): concat([ds1, ds2], dim="q") + with raises_regex(ValueError, "'q' is not present in all datasets"): + concat([ds2, ds1], dim="q") class TestConcatDataset: