Combine='by_coords' and concat dim deprecation in open_mfdataset (#5669)

* deprecate

* test

* whats-new

* Update doc/whats-new.rst

Co-authored-by: Mathias Hauser <mathause@users.noreply.github.com>

Co-authored-by: Maximilian Roos <5635139+max-sixty@users.noreply.github.com>
Co-authored-by: Mathias Hauser <mathause@users.noreply.github.com>

Author: 3 people

doc/whats-new.rst

@@ -56,6 +56,9 @@ Breaking changes
 Deprecations
 ~~~~~~~~~~~~
 
+- :py:meth:`xarray.open_mfdataset` will now error instead of warn when a value for ``concat_dim`` is
+  passed alongside ``combine='by_coords'``. By `Tom Nicholas <https://github.com/TomNicholas>`_.
+
 
 Bug fixes
 ~~~~~~~~~

xarray/backends/api.py

@@ -1,5 +1,4 @@
 import os
-import warnings
 from glob import glob
 from io import BytesIO
 from numbers import Number
@@ -885,15 +884,11 @@ def open_mfdataset(
             list(combined_ids_paths.keys()),
             list(combined_ids_paths.values()),
         )
-
-    # TODO raise an error instead of a warning after v0.19
     elif combine == "by_coords" and concat_dim is not None:
-        warnings.warn(
+        raise ValueError(
             "When combine='by_coords', passing a value for `concat_dim` has no "
-            "effect. This combination will raise an error in future. To manually "
-            "combine along a specific dimension you should instead specify "
-            "combine='nested' along with a value for `concat_dim`.",
-            DeprecationWarning,
+            "effect. To manually combine along a specific dimension you should "
+            "instead specify combine='nested' along with a value for `concat_dim`.",
         )
 
     open_kwargs = dict(engine=engine, chunks=chunks or {}, **kwargs)

xarray/tests/test_backends.py

@@ -3497,17 +3497,14 @@ def test_open_mfdataset_auto_combine(self):
                 with open_mfdataset([tmp2, tmp1], combine="by_coords") as actual:
                     assert_identical(original, actual)
 
-    # TODO check for an error instead of a warning once deprecated
     def test_open_mfdataset_raise_on_bad_combine_args(self):
         # Regression test for unhelpful error shown in #5230
         original = Dataset({"foo": ("x", np.random.randn(10)), "x": np.arange(10)})
         with create_tmp_file() as tmp1:
             with create_tmp_file() as tmp2:
                 original.isel(x=slice(5)).to_netcdf(tmp1)
                 original.isel(x=slice(5, 10)).to_netcdf(tmp2)
-                with pytest.warns(
-                    DeprecationWarning, match="`concat_dim` has no effect"
-                ):
+                with pytest.raises(ValueError, match="`concat_dim` has no effect"):
                     open_mfdataset([tmp1, tmp2], concat_dim="x")
 
     @pytest.mark.xfail(reason="mfdataset loses encoding currently.")