Repository: pydata/xarray

Issue:
cbar_kwargs for faceting plots
I'm using faceting and the add_colorbar=True argument along with cbar_kwargs. This triggers the following error message:

```
ValueError: cbar_ax and cbar_kwargs can't be used with add_colorbar=False.
```

I searched the facetgrid.py module and found the following lines:
```python
func_kwargs = kwargs.copy()
func_kwargs.update(cmap_params)
func_kwargs.update({'add_colorbar': False, 'add_labels': False})
```
I realized that if add_colorbar is True, then the facet function draws one colorbar for each subplot. My initial intention was providing cbar_kwargs to reduce the whitespace between the colorbar en the right edge of the plot.

Is there any way to fix this?

Discussion:
pep8speaks: Hello @dcherian! Thanks for updating the PR.

 - There are no PEP8 issues in the file [`xarray/plot/facetgrid.py`](https://github.com/dcherian/xarray/blob/3455a1c7162422adbb1bdb55eb77fa5ad29e0842/xarray/plot/facetgrid.py) !

 - There are no PEP8 issues in the file [`xarray/tests/test_plot.py`](https://github.com/dcherian/xarray/blob/3455a1c7162422adbb1bdb55eb77fa5ad29e0842/xarray/tests/test_plot.py) !



Patch:
diff --git a/doc/plotting.rst b/doc/plotting.rst
index 6e3498c1..95e63cbf 100644
--- a/doc/plotting.rst
+++ b/doc/plotting.rst
@@ -527,7 +527,8 @@ Faceted plotting supports other arguments common to xarray 2d plots.
 
     @savefig plot_facet_robust.png
     g = hasoutliers.plot.pcolormesh('lon', 'lat', col='time', col_wrap=3,
-                                    robust=True, cmap='viridis')
+                                    robust=True, cmap='viridis',
+				     cbar_kwargs={'label': 'this has outliers'})
 
 FacetGrid Objects
 ~~~~~~~~~~~~~~~~~
diff --git a/doc/whats-new.rst b/doc/whats-new.rst
index 6512d48d..4ae498c3 100644
--- a/doc/whats-new.rst
+++ b/doc/whats-new.rst
@@ -33,6 +33,9 @@ v0.11.0 (unreleased)
 Breaking changes
 ~~~~~~~~~~~~~~~~
 
+- Iterating over a ``Dataset`` now includes only data variables, not coordinates.
+  Similarily, calling ``len`` and ``bool`` on a ``Dataset`` now  
+  includes only data variables
 - Xarray's storage backends now automatically open and close files when
   necessary, rather than requiring opening a file with ``autoclose=True``. A
   global least-recently-used cache is used to store open files; the default
@@ -83,6 +86,9 @@ Enhancements
 Bug fixes
 ~~~~~~~~~
 
+- ``FacetGrid`` now properly uses the ``cbar_kwargs`` keyword argument.
+  (:issue:`1504`, :issue:`1717`)
+  By `Deepak Cherian <https://github.com/dcherian>`_.
 - Addition and subtraction operators used with a CFTimeIndex now preserve the
   index's type. (:issue:`2244`).
   By `Spencer Clark <https://github.com/spencerkclark>`_.
diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py
index 756cd795..534029b9 100644
--- a/xarray/core/dataset.py
+++ b/xarray/core/dataset.py
@@ -930,32 +930,13 @@ class Dataset(Mapping, ImplementsDatasetReduce, DataWithCoords,
         return key in self._variables
 
     def __len__(self):
-        warnings.warn('calling len() on an xarray.Dataset will change in '
-                      'xarray v0.11 to only include data variables, not '
-                      'coordinates. Call len() on the Dataset.variables '
-                      'property instead, like ``len(ds.variables)``, to '
-                      'preserve existing behavior in a forwards compatible '
-                      'manner.',
-                      FutureWarning, stacklevel=2)
-        return len(self._variables)
+        return len(self.data_vars)
 
     def __bool__(self):
-        warnings.warn('casting an xarray.Dataset to a boolean will change in '
-                      'xarray v0.11 to only include data variables, not '
-                      'coordinates. Cast the Dataset.variables property '
-                      'instead to preserve existing behavior in a forwards '
-                      'compatible manner.',
-                      FutureWarning, stacklevel=2)
-        return bool(self._variables)
+        return bool(self.data_vars)
 
     def __iter__(self):
-        warnings.warn('iteration over an xarray.Dataset will change in xarray '
-                      'v0.11 to only include data variables, not coordinates. '
-                      'Iterate over the Dataset.variables property instead to '
-                      'preserve existing behavior in a forwards compatible '
-                      'manner.',
-                      FutureWarning, stacklevel=2)
-        return iter(self._variables)
+        return iter(self.data_vars)
 
     def __array__(self, dtype=None):
         raise TypeError('cannot directly convert an xarray.Dataset into a '
diff --git a/xarray/plot/facetgrid.py b/xarray/plot/facetgrid.py
index 32a954a3..f133e780 100644
--- a/xarray/plot/facetgrid.py
+++ b/xarray/plot/facetgrid.py
@@ -223,6 +223,11 @@ class FacetGrid(object):
 
         cmapkw = kwargs.get('cmap')
         colorskw = kwargs.get('colors')
+        cbar_kwargs = kwargs.pop('cbar_kwargs', {})
+        cbar_kwargs = {} if cbar_kwargs is None else dict(cbar_kwargs)
+
+        if kwargs.get('cbar_ax', None) is not None:
+            raise ValueError('cbar_ax not supported by FacetGrid.')
 
         # colors is mutually exclusive with cmap
         if cmapkw and colorskw:
@@ -264,7 +269,7 @@ class FacetGrid(object):
         self._finalize_grid(x, y)
 
         if kwargs.get('add_colorbar', True):
-            self.add_colorbar()
+            self.add_colorbar(**cbar_kwargs)
 
         return self
 


Test patch:
diff --git a/xarray/tests/test_dataset.py b/xarray/tests/test_dataset.py
index 89704653..adc80966 100644
--- a/xarray/tests/test_dataset.py
+++ b/xarray/tests/test_dataset.py
@@ -420,16 +420,12 @@ class TestDataset(object):
         assert isinstance(ds.dims.mapping, utils.SortedKeysDict)
         assert type(ds.dims.mapping.mapping) is dict  # noqa
 
-        with pytest.warns(FutureWarning):
-            assert list(ds) == list(ds.variables)
-        with pytest.warns(FutureWarning):
-            assert list(ds.keys()) == list(ds.variables)
+        assert list(ds) == list(ds.data_vars)
+        assert list(ds.keys()) == list(ds.data_vars)
         assert 'aasldfjalskdfj' not in ds.variables
         assert 'dim1' in repr(ds.variables)
-        with pytest.warns(FutureWarning):
-            assert len(ds) == 7
-        with pytest.warns(FutureWarning):
-            assert bool(ds)
+        assert len(ds) == 3
+        assert bool(ds)
 
         assert list(ds.data_vars) == ['var1', 'var2', 'var3']
         assert list(ds.data_vars.keys()) == ['var1', 'var2', 'var3']
diff --git a/xarray/tests/test_plot.py b/xarray/tests/test_plot.py
index 30698874..38cf68e4 100644
--- a/xarray/tests/test_plot.py
+++ b/xarray/tests/test_plot.py
@@ -1147,6 +1147,23 @@ class Common2dMixin(object):
         # check that all colormaps are the same
         assert len(set(m.get_cmap().name for m in fg._mappables)) == 1
 
+    def test_facetgrid_cbar_kwargs(self):
+        a = easy_array((10, 15, 2, 3))
+        d = DataArray(a, dims=['y', 'x', 'columns', 'rows'])
+        g = self.plotfunc(d, x='x', y='y', col='columns', row='rows',
+                          cbar_kwargs={'label': 'test_label'})
+
+        # catch contour case
+        if hasattr(g, 'cbar'):
+            assert g.cbar._label == 'test_label'
+
+    def test_facetgrid_no_cbar_ax(self):
+        a = easy_array((10, 15, 2, 3))
+        d = DataArray(a, dims=['y', 'x', 'columns', 'rows'])
+        with pytest.raises(ValueError):
+            g = self.plotfunc(d, x='x', y='y', col='columns', row='rows',
+                              cbar_ax=1)
+
     def test_cmap_and_color_both(self):
         with pytest.raises(ValueError):
             self.plotmethod(colors='k', cmap='RdBu')