From bab749175d1c1aa9664e64e79c5fd6d44cba2e75 Mon Sep 17 00:00:00 2001 From: Richard Date: Sun, 12 Apr 2020 08:29:48 -0400 Subject: [PATCH 01/15] BUG: groupby.hist legend should use group keys --- pandas/plotting/_core.py | 8 ++++ pandas/plotting/_matplotlib/hist.py | 28 ++++++++++++- pandas/tests/plotting/test_groupby.py | 18 +++++++- pandas/tests/plotting/test_hist_method.py | 51 ++++++++++++++++++++++- 4 files changed, 102 insertions(+), 3 deletions(-) diff --git a/pandas/plotting/_core.py b/pandas/plotting/_core.py index 467bdf7e0745d..647e932614b52 100644 --- a/pandas/plotting/_core.py +++ b/pandas/plotting/_core.py @@ -21,6 +21,7 @@ def hist_series( yrot=None, figsize=None, bins=10, + legend=False, backend=None, **kwargs, ): @@ -50,6 +51,8 @@ def hist_series( bin edges are calculated and returned. If bins is a sequence, gives bin edges, including left edge of first bin and right edge of last bin. In this case, bins is returned unmodified. + legend : bool, default False + Whether to show the legend. backend : str, default None Backend to use instead of the backend specified in the option ``plotting.backend``. For instance, 'matplotlib'. Alternatively, to @@ -82,6 +85,7 @@ def hist_series( yrot=yrot, figsize=figsize, bins=bins, + legend=legend, **kwargs, ) @@ -101,6 +105,7 @@ def hist_frame( figsize=None, layout=None, bins=10, + legend=False, backend=None, **kwargs, ): @@ -154,6 +159,8 @@ def hist_frame( bin edges are calculated and returned. If bins is a sequence, gives bin edges, including left edge of first bin and right edge of last bin. In this case, bins is returned unmodified. + legend : bool, default False + Whether to show the legend. backend : str, default None Backend to use instead of the backend specified in the option ``plotting.backend``. For instance, 'matplotlib'. Alternatively, to @@ -203,6 +210,7 @@ def hist_frame( sharey=sharey, figsize=figsize, layout=layout, + legend=legend, bins=bins, **kwargs, ) diff --git a/pandas/plotting/_matplotlib/hist.py b/pandas/plotting/_matplotlib/hist.py index b0ce43dc2eb36..fae6a407398ad 100644 --- a/pandas/plotting/_matplotlib/hist.py +++ b/pandas/plotting/_matplotlib/hist.py @@ -225,6 +225,7 @@ def _grouped_hist( xrot=None, ylabelsize=None, yrot=None, + legend=False, **kwargs, ): """ @@ -243,6 +244,7 @@ def _grouped_hist( sharey : bool, default False rot : int, default 90 grid : bool, default True + legend: : bool, default False kwargs : dict, keyword arguments passed to matplotlib.Axes.hist Returns @@ -250,8 +252,19 @@ def _grouped_hist( collection of Matplotlib Axes """ + if legend and "label" not in kwargs: + if isinstance(data, ABCDataFrame): + if column is None: + kwargs["label"] = data.columns + else: + kwargs["label"] = column + else: + kwargs["label"] = data.name + def plot_group(group, ax): ax.hist(group.dropna().values, bins=bins, **kwargs) + if legend: + ax.legend() if xrot is None: xrot = rot @@ -290,6 +303,7 @@ def hist_series( yrot=None, figsize=None, bins=10, + legend=False, **kwds, ): import matplotlib.pyplot as plt @@ -308,8 +322,11 @@ def hist_series( elif ax.get_figure() != fig: raise AssertionError("passed axis not bound to passed figure") values = self.dropna().values - + if legend and "label" not in kwds: + kwds["label"] = self.name ax.hist(values, bins=bins, **kwds) + if legend: + ax.legend() ax.grid(grid) axes = np.array([ax]) @@ -334,6 +351,7 @@ def hist_series( xrot=xrot, ylabelsize=ylabelsize, yrot=yrot, + legend=legend, **kwds, ) @@ -358,6 +376,7 @@ def hist_frame( figsize=None, layout=None, bins=10, + legend=False, **kwds, ): if by is not None: @@ -376,6 +395,7 @@ def hist_frame( xrot=xrot, ylabelsize=ylabelsize, yrot=yrot, + legend=legend, **kwds, ) return axes @@ -401,11 +421,17 @@ def hist_frame( ) _axes = _flatten(axes) + can_set_label = "label" not in kwds + for i, col in enumerate(data.columns): ax = _axes[i] + if legend and can_set_label: + kwds["label"] = col ax.hist(data[col].dropna().values, bins=bins, **kwds) ax.set_title(col) ax.grid(grid) + if legend: + ax.legend() _set_ticks_props( axes, xlabelsize=xlabelsize, xrot=xrot, ylabelsize=ylabelsize, yrot=yrot diff --git a/pandas/tests/plotting/test_groupby.py b/pandas/tests/plotting/test_groupby.py index 238639bd3732d..88c237cecc1bb 100644 --- a/pandas/tests/plotting/test_groupby.py +++ b/pandas/tests/plotting/test_groupby.py @@ -2,10 +2,11 @@ import numpy as np +import pytest import pandas.util._test_decorators as td -from pandas import DataFrame, Series +from pandas import DataFrame, Index, Series import pandas._testing as tm from pandas.tests.plotting.common import TestPlotBase @@ -65,3 +66,18 @@ def test_plot_kwargs(self): res = df.groupby("z").plot.scatter(x="x", y="y") assert len(res["a"].collections) == 1 + + +@td.skip_if_no_mpl +@pytest.mark.parametrize("column", [None, "b"]) +@pytest.mark.parametrize("label", [None, "d"]) +def test_hist_with_legend(column, label): + index = Index(15 * [1] + 15 * [2], name="c") + df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"]) + g = df.groupby("c") + + g.hist(column=column, label=label, legend=True) + tm.close() + if column != "b": + g["a"].hist(label=label, legend=True) + tm.close() diff --git a/pandas/tests/plotting/test_hist_method.py b/pandas/tests/plotting/test_hist_method.py index 5a30e9fbb91c6..dbf54518cf902 100644 --- a/pandas/tests/plotting/test_hist_method.py +++ b/pandas/tests/plotting/test_hist_method.py @@ -6,7 +6,7 @@ import pandas.util._test_decorators as td -from pandas import DataFrame, Series +from pandas import DataFrame, Index, Series import pandas._testing as tm from pandas.tests.plotting.common import TestPlotBase, _check_plot_works @@ -293,6 +293,28 @@ def test_hist_column_order_unchanged(self, column, expected): assert result == expected + @pytest.mark.slow + @pytest.mark.parametrize("by", [None, "b"]) + @pytest.mark.parametrize("label", [None, "c"]) + def test_hist_with_legend(self, by, label): + expected_labels = label or "a" + expected_axes_num = 1 if by is None else 2 + expected_layout = (1, 1) if by is None else (1, 2) + + index = 15 * [1] + 15 * [2] + s = Series(np.random.randn(30), index=index, name="a") + s.index.name = "b" + + kwargs = {"legend": True, "by": by} + if label is not None: + # Behavior differs if kwargs contains "label": None + kwargs["label"] = label + + _check_plot_works(s.hist, **kwargs) + axes = s.hist(**kwargs) + self._check_axes_shape(axes, axes_num=expected_axes_num, layout=expected_layout) + self._check_legend_labels(axes, expected_labels) + @td.skip_if_no_mpl class TestDataFrameGroupByPlots(TestPlotBase): @@ -484,3 +506,30 @@ def test_axis_share_xy(self): assert ax1._shared_y_axes.joined(ax1, ax2) assert ax2._shared_y_axes.joined(ax1, ax2) + + @pytest.mark.slow + @pytest.mark.parametrize("by", [None, "c"]) + @pytest.mark.parametrize("column", [None, "b"]) + @pytest.mark.parametrize("label", [None, "d"]) + def test_hist_with_legend(self, by, column, label): + expected_axes_num = 1 if by is None and column is not None else 2 + expected_layout = (1, expected_axes_num) + expected_labels = label or column or ["a", "b"] + if by is not None: + expected_labels = [expected_labels] * 2 + + index = Index(15 * [1] + 15 * [2], name="c") + df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"]) + + kwargs = {"legend": True, "by": by, "column": column} + if label is not None: + # Behavior differs if kwargs contains "label": None + kwargs["label"] = label + + _check_plot_works(df.hist, **kwargs) + axes = df.hist(**kwargs) + self._check_axes_shape(axes, axes_num=expected_axes_num, layout=expected_layout) + if by is None: + axes = axes[0] + for expected_label, ax in zip(expected_labels, axes): + self._check_legend_labels(ax, expected_label) From e0c94668df052f590a72a3dc52402039c8792307 Mon Sep 17 00:00:00 2001 From: Richard Date: Sun, 17 May 2020 18:10:24 -0400 Subject: [PATCH 02/15] Some changes --- pandas/plotting/_matplotlib/hist.py | 4 +- pandas/tests/plotting/common.py | 1 + pandas/tests/plotting/test_groupby.py | 51 +++++++++++----- pandas/tests/plotting/test_hist_method.py | 74 +++++++++++------------ 4 files changed, 74 insertions(+), 56 deletions(-) diff --git a/pandas/plotting/_matplotlib/hist.py b/pandas/plotting/_matplotlib/hist.py index fae6a407398ad..962f222676a72 100644 --- a/pandas/plotting/_matplotlib/hist.py +++ b/pandas/plotting/_matplotlib/hist.py @@ -303,7 +303,7 @@ def hist_series( yrot=None, figsize=None, bins=10, - legend=False, + legend: bool = False, **kwds, ): import matplotlib.pyplot as plt @@ -376,7 +376,7 @@ def hist_frame( figsize=None, layout=None, bins=10, - legend=False, + legend: bool = False, **kwds, ): if by is not None: diff --git a/pandas/tests/plotting/common.py b/pandas/tests/plotting/common.py index f2f7b37170ec9..34e7e31b0d38b 100644 --- a/pandas/tests/plotting/common.py +++ b/pandas/tests/plotting/common.py @@ -246,6 +246,7 @@ def _check_text_labels(self, texts, expected): assert texts.get_text() == expected else: labels = [t.get_text() for t in texts] + print(labels, expected) assert len(labels) == len(expected) for label, e in zip(labels, expected): assert label == e diff --git a/pandas/tests/plotting/test_groupby.py b/pandas/tests/plotting/test_groupby.py index 88c237cecc1bb..7fcbfadb61b2d 100644 --- a/pandas/tests/plotting/test_groupby.py +++ b/pandas/tests/plotting/test_groupby.py @@ -8,7 +8,7 @@ from pandas import DataFrame, Index, Series import pandas._testing as tm -from pandas.tests.plotting.common import TestPlotBase +from pandas.tests.plotting.common import TestPlotBase, _check_plot_works @td.skip_if_no_mpl @@ -67,17 +67,40 @@ def test_plot_kwargs(self): res = df.groupby("z").plot.scatter(x="x", y="y") assert len(res["a"].collections) == 1 - -@td.skip_if_no_mpl -@pytest.mark.parametrize("column", [None, "b"]) -@pytest.mark.parametrize("label", [None, "d"]) -def test_hist_with_legend(column, label): - index = Index(15 * [1] + 15 * [2], name="c") - df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"]) - g = df.groupby("c") - - g.hist(column=column, label=label, legend=True) - tm.close() - if column != "b": - g["a"].hist(label=label, legend=True) + @pytest.mark.parametrize("column, expected_axes_num", [(None, 2), ("b", 1)]) + @pytest.mark.parametrize("label", [None, "d"]) + def test_groupby_hist_with_legend(self, column, expected_axes_num, label): + expected_layout = (1, expected_axes_num) + expected_labels = label or column or [["a"], ["b"]] + + index = Index(15 * [1] + 15 * [2], name="c") + df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"]) + g = df.groupby("c") + + kwargs = {"legend": True, "column": column} + # Don't add "label": None, causes different behavior than no label kwarg + if label is not None: + kwargs["label"] = label + + ret = g.hist(**kwargs) + for (_, axes) in ret.iteritems(): + self._check_axes_shape(axes, axes_num=expected_axes_num, layout=expected_layout) + for ax, expected_label in zip(axes[0], expected_labels): + self._check_legend_labels(ax, expected_label) tm.close() + + @pytest.mark.parametrize("label, expected_label", [(None, ['1', '2']), ("d", ["d", "d"])]) + def test_groupby_hist_series_with_legend(self, label, expected_label): + index = Index(15 * [1] + 15 * [2], name="c") + df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"]) + g = df.groupby("c") + + kwargs = {"legend": True} + # Don't add "label": None, causes different behavior than no label kwarg + if label is not None: + kwargs["label"] = label + + axes = g["a"].hist(**kwargs) + for (_, ax) in axes.iteritems(): + self._check_axes_shape(ax, axes_num=1, layout=(1, 1)) + self._check_legend_labels(ax, expected_label) diff --git a/pandas/tests/plotting/test_hist_method.py b/pandas/tests/plotting/test_hist_method.py index dbf54518cf902..6069275ec4b26 100644 --- a/pandas/tests/plotting/test_hist_method.py +++ b/pandas/tests/plotting/test_hist_method.py @@ -129,6 +129,23 @@ def test_plot_fails_when_ax_differs_from_figure(self): with pytest.raises(AssertionError): self.ts.hist(ax=ax1, figure=fig2) + @pytest.mark.slow + @pytest.mark.parametrize("by, expected_axes_num, expected_layout", [(None, 1, (1, 1)), ("b", 2, (1, 2))]) + @pytest.mark.parametrize("label, expected_label", [(None, "a"), ("c", "c")]) + def test_hist_with_legend(self, by, expected_axes_num, expected_layout, label, expected_label): + index = 15 * [1] + 15 * [2] + s = Series(np.random.randn(30), index=index, name="a") + s.index.name = "b" + + kwargs = {"legend": True, "by": by} + if label is not None: + # Behavior differs if kwargs contains "label": None + kwargs["label"] = label + + axes = _check_plot_works(s.hist, **kwargs) + self._check_axes_shape(axes, axes_num=expected_axes_num, layout=expected_layout) + self._check_legend_labels(axes, expected_label) + @td.skip_if_no_mpl class TestDataFramePlots(TestPlotBase): @@ -294,26 +311,30 @@ def test_hist_column_order_unchanged(self, column, expected): assert result == expected @pytest.mark.slow - @pytest.mark.parametrize("by", [None, "b"]) - @pytest.mark.parametrize("label", [None, "c"]) - def test_hist_with_legend(self, by, label): - expected_labels = label or "a" - expected_axes_num = 1 if by is None else 2 - expected_layout = (1, 1) if by is None else (1, 2) + @pytest.mark.parametrize("by", [None, "c"]) + @pytest.mark.parametrize("column", [None, "b"]) + @pytest.mark.parametrize("label", [None, "d"]) + def test_hist_with_legend(self, by, column, label): + expected_axes_num = 1 if by is None and column is not None else 2 + expected_layout = (1, expected_axes_num) + expected_labels = label or column or ["a", "b"] + if by is not None: + expected_labels = [expected_labels] * 2 - index = 15 * [1] + 15 * [2] - s = Series(np.random.randn(30), index=index, name="a") - s.index.name = "b" + index = Index(15 * [1] + 15 * [2], name="c") + df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"]) - kwargs = {"legend": True, "by": by} + kwargs = {"legend": True, "by": by, "column": column} if label is not None: # Behavior differs if kwargs contains "label": None kwargs["label"] = label - _check_plot_works(s.hist, **kwargs) - axes = s.hist(**kwargs) + axes = _check_plot_works(df.hist, **kwargs) self._check_axes_shape(axes, axes_num=expected_axes_num, layout=expected_layout) - self._check_legend_labels(axes, expected_labels) + if by is None and column is None and label is None: + axes = axes[0] + for expected_label, ax in zip(expected_labels, axes): + self._check_legend_labels(ax, expected_label) @td.skip_if_no_mpl @@ -506,30 +527,3 @@ def test_axis_share_xy(self): assert ax1._shared_y_axes.joined(ax1, ax2) assert ax2._shared_y_axes.joined(ax1, ax2) - - @pytest.mark.slow - @pytest.mark.parametrize("by", [None, "c"]) - @pytest.mark.parametrize("column", [None, "b"]) - @pytest.mark.parametrize("label", [None, "d"]) - def test_hist_with_legend(self, by, column, label): - expected_axes_num = 1 if by is None and column is not None else 2 - expected_layout = (1, expected_axes_num) - expected_labels = label or column or ["a", "b"] - if by is not None: - expected_labels = [expected_labels] * 2 - - index = Index(15 * [1] + 15 * [2], name="c") - df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"]) - - kwargs = {"legend": True, "by": by, "column": column} - if label is not None: - # Behavior differs if kwargs contains "label": None - kwargs["label"] = label - - _check_plot_works(df.hist, **kwargs) - axes = df.hist(**kwargs) - self._check_axes_shape(axes, axes_num=expected_axes_num, layout=expected_layout) - if by is None: - axes = axes[0] - for expected_label, ax in zip(expected_labels, axes): - self._check_legend_labels(ax, expected_label) From 61693c64ffec05c32cf8496137af7edd378fd5c4 Mon Sep 17 00:00:00 2001 From: Richard Date: Mon, 18 May 2020 16:57:30 -0400 Subject: [PATCH 03/15] Requested changes --- pandas/plotting/_core.py | 56 +++++++++++++---------- pandas/tests/plotting/test_groupby.py | 14 ++++-- pandas/tests/plotting/test_hist_method.py | 18 +++++--- 3 files changed, 56 insertions(+), 32 deletions(-) diff --git a/pandas/plotting/_core.py b/pandas/plotting/_core.py index 647e932614b52..4907ea1412912 100644 --- a/pandas/plotting/_core.py +++ b/pandas/plotting/_core.py @@ -1,4 +1,5 @@ import importlib +from typing import TYPE_CHECKING, Optional, Sequence, Tuple, Union from pandas._config import get_option @@ -9,20 +10,23 @@ from pandas.core.base import PandasObject +if TYPE_CHECKING: + from pandas import DataFrame + def hist_series( self, by=None, ax=None, - grid=True, - xlabelsize=None, - xrot=None, - ylabelsize=None, - yrot=None, - figsize=None, - bins=10, - legend=False, - backend=None, + grid: bool = True, + xlabelsize: Optional[int] = None, + xrot: Optional[float] = None, + ylabelsize: Optional[int] = None, + yrot: Optional[float] = None, + figsize: Optional[Tuple[int, int]] = None, + bins: Union[int, Sequence[int]] = 10, + legend: bool = False, + backend: Optional[str] = None, **kwargs, ): """ @@ -53,6 +57,9 @@ def hist_series( bin. In this case, bins is returned unmodified. legend : bool, default False Whether to show the legend. + + ..versionadded:: 1.1.0 + backend : str, default None Backend to use instead of the backend specified in the option ``plotting.backend``. For instance, 'matplotlib'. Alternatively, to @@ -91,22 +98,22 @@ def hist_series( def hist_frame( - data, - column=None, + data: "DataFrame", + column: Optional[Union[str, Sequence[str]]] = None, by=None, - grid=True, - xlabelsize=None, - xrot=None, - ylabelsize=None, - yrot=None, + grid: bool = True, + xlabelsize: Optional[int] = None, + xrot: Optional[float] = None, + ylabelsize: Optional[int] = None, + yrot: Optional[float] = None, ax=None, - sharex=False, - sharey=False, - figsize=None, - layout=None, - bins=10, - legend=False, - backend=None, + sharex: bool = False, + sharey: bool = False, + figsize: Optional[Tuple[int, int]] = None, + layout: Optional[Tuple[int, int]] = None, + bins: Union[int, Sequence[int]] = 10, + legend: bool = False, + backend: Optional[str] = None, **kwargs, ): """ @@ -161,6 +168,9 @@ def hist_frame( bin. In this case, bins is returned unmodified. legend : bool, default False Whether to show the legend. + + ..versionadded:: 1.1.0 + backend : str, default None Backend to use instead of the backend specified in the option ``plotting.backend``. For instance, 'matplotlib'. Alternatively, to diff --git a/pandas/tests/plotting/test_groupby.py b/pandas/tests/plotting/test_groupby.py index 7fcbfadb61b2d..656201f4971dc 100644 --- a/pandas/tests/plotting/test_groupby.py +++ b/pandas/tests/plotting/test_groupby.py @@ -8,7 +8,7 @@ from pandas import DataFrame, Index, Series import pandas._testing as tm -from pandas.tests.plotting.common import TestPlotBase, _check_plot_works +from pandas.tests.plotting.common import TestPlotBase @td.skip_if_no_mpl @@ -70,6 +70,8 @@ def test_plot_kwargs(self): @pytest.mark.parametrize("column, expected_axes_num", [(None, 2), ("b", 1)]) @pytest.mark.parametrize("label", [None, "d"]) def test_groupby_hist_with_legend(self, column, expected_axes_num, label): + # GH 6279 + # Histogram can have a legend expected_layout = (1, expected_axes_num) expected_labels = label or column or [["a"], ["b"]] @@ -84,13 +86,19 @@ def test_groupby_hist_with_legend(self, column, expected_axes_num, label): ret = g.hist(**kwargs) for (_, axes) in ret.iteritems(): - self._check_axes_shape(axes, axes_num=expected_axes_num, layout=expected_layout) + self._check_axes_shape( + axes, axes_num=expected_axes_num, layout=expected_layout + ) for ax, expected_label in zip(axes[0], expected_labels): self._check_legend_labels(ax, expected_label) tm.close() - @pytest.mark.parametrize("label, expected_label", [(None, ['1', '2']), ("d", ["d", "d"])]) + @pytest.mark.parametrize( + "label, expected_label", [(None, ["1", "2"]), ("d", ["d", "d"])] + ) def test_groupby_hist_series_with_legend(self, label, expected_label): + # GH 6279 + # Histogram can have a legend index = Index(15 * [1] + 15 * [2], name="c") df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"]) g = df.groupby("c") diff --git a/pandas/tests/plotting/test_hist_method.py b/pandas/tests/plotting/test_hist_method.py index 6069275ec4b26..9bb8db0b85c56 100644 --- a/pandas/tests/plotting/test_hist_method.py +++ b/pandas/tests/plotting/test_hist_method.py @@ -129,17 +129,22 @@ def test_plot_fails_when_ax_differs_from_figure(self): with pytest.raises(AssertionError): self.ts.hist(ax=ax1, figure=fig2) - @pytest.mark.slow - @pytest.mark.parametrize("by, expected_axes_num, expected_layout", [(None, 1, (1, 1)), ("b", 2, (1, 2))]) + @pytest.mark.parametrize( + "by, expected_axes_num, expected_layout", [(None, 1, (1, 1)), ("b", 2, (1, 2))] + ) @pytest.mark.parametrize("label, expected_label", [(None, "a"), ("c", "c")]) - def test_hist_with_legend(self, by, expected_axes_num, expected_layout, label, expected_label): + def test_hist_with_legend( + self, by, expected_axes_num, expected_layout, label, expected_label + ): + # GH 6279 + # Histogram can have a legend index = 15 * [1] + 15 * [2] s = Series(np.random.randn(30), index=index, name="a") s.index.name = "b" kwargs = {"legend": True, "by": by} + # We get warnings if kwargs contains "label": None if label is not None: - # Behavior differs if kwargs contains "label": None kwargs["label"] = label axes = _check_plot_works(s.hist, **kwargs) @@ -310,11 +315,12 @@ def test_hist_column_order_unchanged(self, column, expected): assert result == expected - @pytest.mark.slow @pytest.mark.parametrize("by", [None, "c"]) @pytest.mark.parametrize("column", [None, "b"]) @pytest.mark.parametrize("label", [None, "d"]) def test_hist_with_legend(self, by, column, label): + # GH 6279 + # Histogram can have a legend expected_axes_num = 1 if by is None and column is not None else 2 expected_layout = (1, expected_axes_num) expected_labels = label or column or ["a", "b"] @@ -325,8 +331,8 @@ def test_hist_with_legend(self, by, column, label): df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"]) kwargs = {"legend": True, "by": by, "column": column} + # We get warnings if kwargs contains "label": None if label is not None: - # Behavior differs if kwargs contains "label": None kwargs["label"] = label axes = _check_plot_works(df.hist, **kwargs) From cbfc167b25eae4c6030dc1f0b6c0544f454afc4c Mon Sep 17 00:00:00 2001 From: Richard Date: Mon, 18 May 2020 17:06:59 -0400 Subject: [PATCH 04/15] whatsnew --- doc/source/whatsnew/v1.1.0.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v1.1.0.rst b/doc/source/whatsnew/v1.1.0.rst index 84ad478226175..4a03a423ece44 100644 --- a/doc/source/whatsnew/v1.1.0.rst +++ b/doc/source/whatsnew/v1.1.0.rst @@ -183,7 +183,7 @@ Other enhancements - :meth:`~pandas.core.resample.Resampler.interpolate` now supports SciPy interpolation method :class:`scipy.interpolate.CubicSpline` as method ``cubicspline`` (:issue:`33670`) - The ``ExtensionArray`` class has now an :meth:`~pandas.arrays.ExtensionArray.equals` method, similarly to :meth:`Series.equals` (:issue:`27081`). -- +- :meth:`DataFrame.hist`, :meth:`Series.hist`, :meth:`DataFrameGroupby.hist`, and :meth:`SeriesGroupby.hist` have gained the ``legend`` argument. Set to True to show a legend in the histogram. (:issue:`6279`) .. --------------------------------------------------------------------------- From 726d14797f92354e89159afa45d102a44214d640 Mon Sep 17 00:00:00 2001 From: Richard Date: Mon, 18 May 2020 17:16:14 -0400 Subject: [PATCH 05/15] Minor tweaks --- pandas/tests/plotting/common.py | 1 - pandas/tests/plotting/test_groupby.py | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/pandas/tests/plotting/common.py b/pandas/tests/plotting/common.py index 34e7e31b0d38b..f2f7b37170ec9 100644 --- a/pandas/tests/plotting/common.py +++ b/pandas/tests/plotting/common.py @@ -246,7 +246,6 @@ def _check_text_labels(self, texts, expected): assert texts.get_text() == expected else: labels = [t.get_text() for t in texts] - print(labels, expected) assert len(labels) == len(expected) for label, e in zip(labels, expected): assert label == e diff --git a/pandas/tests/plotting/test_groupby.py b/pandas/tests/plotting/test_groupby.py index 656201f4971dc..8309788b690dc 100644 --- a/pandas/tests/plotting/test_groupby.py +++ b/pandas/tests/plotting/test_groupby.py @@ -80,7 +80,7 @@ def test_groupby_hist_with_legend(self, column, expected_axes_num, label): g = df.groupby("c") kwargs = {"legend": True, "column": column} - # Don't add "label": None, causes different behavior than no label kwarg + # We get warnings if kwargs contains "label": None if label is not None: kwargs["label"] = label @@ -104,7 +104,7 @@ def test_groupby_hist_series_with_legend(self, label, expected_label): g = df.groupby("c") kwargs = {"legend": True} - # Don't add "label": None, causes different behavior than no label kwarg + # We get warnings if kwargs contains "label": None if label is not None: kwargs["label"] = label From 3e4925d5b67479ba338e9bb4091f3a124e507cd4 Mon Sep 17 00:00:00 2001 From: Richard Date: Mon, 18 May 2020 18:25:32 -0400 Subject: [PATCH 06/15] Changed test to use strings --- pandas/tests/plotting/test_groupby.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pandas/tests/plotting/test_groupby.py b/pandas/tests/plotting/test_groupby.py index 8309788b690dc..f5c4446e35e77 100644 --- a/pandas/tests/plotting/test_groupby.py +++ b/pandas/tests/plotting/test_groupby.py @@ -101,6 +101,8 @@ def test_groupby_hist_series_with_legend(self, label, expected_label): # Histogram can have a legend index = Index(15 * [1] + 15 * [2], name="c") df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"]) + # older version of matplotlib fails if labels are not strings + df = df.astype(str) g = df.groupby("c") kwargs = {"legend": True} From a46609aa1f5bd0de8d1f9ac898e5e157e21719bf Mon Sep 17 00:00:00 2001 From: Richard Date: Mon, 18 May 2020 20:09:42 -0400 Subject: [PATCH 07/15] Corrected test modification --- pandas/tests/plotting/test_groupby.py | 6 ++---- pandas/tests/plotting/test_hist_method.py | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/pandas/tests/plotting/test_groupby.py b/pandas/tests/plotting/test_groupby.py index f5c4446e35e77..203722afe6bbc 100644 --- a/pandas/tests/plotting/test_groupby.py +++ b/pandas/tests/plotting/test_groupby.py @@ -75,7 +75,7 @@ def test_groupby_hist_with_legend(self, column, expected_axes_num, label): expected_layout = (1, expected_axes_num) expected_labels = label or column or [["a"], ["b"]] - index = Index(15 * [1] + 15 * [2], name="c") + index = Index(15 * ["1"] + 15 * ["2"], name="c") df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"]) g = df.groupby("c") @@ -99,10 +99,8 @@ def test_groupby_hist_with_legend(self, column, expected_axes_num, label): def test_groupby_hist_series_with_legend(self, label, expected_label): # GH 6279 # Histogram can have a legend - index = Index(15 * [1] + 15 * [2], name="c") + index = Index(15 * ["1"] + 15 * ["2"], name="c") df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"]) - # older version of matplotlib fails if labels are not strings - df = df.astype(str) g = df.groupby("c") kwargs = {"legend": True} diff --git a/pandas/tests/plotting/test_hist_method.py b/pandas/tests/plotting/test_hist_method.py index 9bb8db0b85c56..354aeea216e0a 100644 --- a/pandas/tests/plotting/test_hist_method.py +++ b/pandas/tests/plotting/test_hist_method.py @@ -138,7 +138,7 @@ def test_hist_with_legend( ): # GH 6279 # Histogram can have a legend - index = 15 * [1] + 15 * [2] + index = 15 * ["1"] + 15 * ["2"] s = Series(np.random.randn(30), index=index, name="a") s.index.name = "b" @@ -327,7 +327,7 @@ def test_hist_with_legend(self, by, column, label): if by is not None: expected_labels = [expected_labels] * 2 - index = Index(15 * [1] + 15 * [2], name="c") + index = Index(15 * ["1"] + 15 * ["2"], name="c") df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"]) kwargs = {"legend": True, "by": by, "column": column} From c36cae2af2b50aee23797b71853f6da23ec1c57e Mon Sep 17 00:00:00 2001 From: Richard Date: Tue, 19 May 2020 18:01:23 -0400 Subject: [PATCH 08/15] Cleanup --- pandas/tests/plotting/test_groupby.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pandas/tests/plotting/test_groupby.py b/pandas/tests/plotting/test_groupby.py index 203722afe6bbc..7c3b554e40ff1 100644 --- a/pandas/tests/plotting/test_groupby.py +++ b/pandas/tests/plotting/test_groupby.py @@ -91,7 +91,6 @@ def test_groupby_hist_with_legend(self, column, expected_axes_num, label): ) for ax, expected_label in zip(axes[0], expected_labels): self._check_legend_labels(ax, expected_label) - tm.close() @pytest.mark.parametrize( "label, expected_label", [(None, ["1", "2"]), ("d", ["d", "d"])] From c01c7ab6310c9325004338eabb34a7ef8cc5ee2c Mon Sep 17 00:00:00 2001 From: Richard Date: Thu, 21 May 2020 07:50:22 -0400 Subject: [PATCH 09/15] Test refinements --- pandas/tests/plotting/test_groupby.py | 14 +++++--------- pandas/tests/plotting/test_hist_method.py | 6 ++---- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/pandas/tests/plotting/test_groupby.py b/pandas/tests/plotting/test_groupby.py index 7c3b554e40ff1..1a5efc96c88f6 100644 --- a/pandas/tests/plotting/test_groupby.py +++ b/pandas/tests/plotting/test_groupby.py @@ -69,9 +69,8 @@ def test_plot_kwargs(self): @pytest.mark.parametrize("column, expected_axes_num", [(None, 2), ("b", 1)]) @pytest.mark.parametrize("label", [None, "d"]) - def test_groupby_hist_with_legend(self, column, expected_axes_num, label): - # GH 6279 - # Histogram can have a legend + def test_groupby_hist_frame_with_legend(self, column, expected_axes_num, label): + # GH 6279 - Histogram can have a legend expected_layout = (1, expected_axes_num) expected_labels = label or column or [["a"], ["b"]] @@ -84,8 +83,7 @@ def test_groupby_hist_with_legend(self, column, expected_axes_num, label): if label is not None: kwargs["label"] = label - ret = g.hist(**kwargs) - for (_, axes) in ret.iteritems(): + for axes in g.hist(**kwargs): self._check_axes_shape( axes, axes_num=expected_axes_num, layout=expected_layout ) @@ -96,8 +94,7 @@ def test_groupby_hist_with_legend(self, column, expected_axes_num, label): "label, expected_label", [(None, ["1", "2"]), ("d", ["d", "d"])] ) def test_groupby_hist_series_with_legend(self, label, expected_label): - # GH 6279 - # Histogram can have a legend + # GH 6279 - Histogram can have a legend index = Index(15 * ["1"] + 15 * ["2"], name="c") df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"]) g = df.groupby("c") @@ -107,7 +104,6 @@ def test_groupby_hist_series_with_legend(self, label, expected_label): if label is not None: kwargs["label"] = label - axes = g["a"].hist(**kwargs) - for (_, ax) in axes.iteritems(): + for ax in g["a"].hist(**kwargs): self._check_axes_shape(ax, axes_num=1, layout=(1, 1)) self._check_legend_labels(ax, expected_label) diff --git a/pandas/tests/plotting/test_hist_method.py b/pandas/tests/plotting/test_hist_method.py index 354aeea216e0a..5764f5e96423e 100644 --- a/pandas/tests/plotting/test_hist_method.py +++ b/pandas/tests/plotting/test_hist_method.py @@ -136,8 +136,7 @@ def test_plot_fails_when_ax_differs_from_figure(self): def test_hist_with_legend( self, by, expected_axes_num, expected_layout, label, expected_label ): - # GH 6279 - # Histogram can have a legend + # GH 6279 - Histogram can have a legend index = 15 * ["1"] + 15 * ["2"] s = Series(np.random.randn(30), index=index, name="a") s.index.name = "b" @@ -319,8 +318,7 @@ def test_hist_column_order_unchanged(self, column, expected): @pytest.mark.parametrize("column", [None, "b"]) @pytest.mark.parametrize("label", [None, "d"]) def test_hist_with_legend(self, by, column, label): - # GH 6279 - # Histogram can have a legend + # GH 6279 - Histogram can have a legend expected_axes_num = 1 if by is None and column is not None else 2 expected_layout = (1, expected_axes_num) expected_labels = label or column or ["a", "b"] From c207ca2ba87db999d6368ff5a2c8bcec7b7da1bb Mon Sep 17 00:00:00 2001 From: Richard Date: Thu, 21 May 2020 14:19:12 -0400 Subject: [PATCH 10/15] Updated type-hint to Label --- pandas/plotting/_core.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pandas/plotting/_core.py b/pandas/plotting/_core.py index 4907ea1412912..149ab2b9070f4 100644 --- a/pandas/plotting/_core.py +++ b/pandas/plotting/_core.py @@ -3,6 +3,8 @@ from pandas._config import get_option +from pandas._typing import Label + from pandas.util._decorators import Appender, Substitution from pandas.core.dtypes.common import is_integer, is_list_like @@ -10,6 +12,7 @@ from pandas.core.base import PandasObject + if TYPE_CHECKING: from pandas import DataFrame @@ -99,7 +102,7 @@ def hist_series( def hist_frame( data: "DataFrame", - column: Optional[Union[str, Sequence[str]]] = None, + column: Union[Label, Sequence[Label]] = None, by=None, grid: bool = True, xlabelsize: Optional[int] = None, From 5b9cae7687c6b08ed159b6717e9c011300c3efbf Mon Sep 17 00:00:00 2001 From: Richard Date: Thu, 21 May 2020 14:25:49 -0400 Subject: [PATCH 11/15] Changed type-hint to Label --- pandas/plotting/_core.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/pandas/plotting/_core.py b/pandas/plotting/_core.py index 4447ffe716ce0..2c6ecb3a43fc3 100644 --- a/pandas/plotting/_core.py +++ b/pandas/plotting/_core.py @@ -4,7 +4,6 @@ from pandas._config import get_option from pandas._typing import Label - from pandas.util._decorators import Appender, Substitution from pandas.core.dtypes.common import is_integer, is_list_like @@ -12,7 +11,6 @@ from pandas.core.base import PandasObject - if TYPE_CHECKING: from pandas import DataFrame From 1db3b35b17e0156ca9b4a8ef4f855e700199a7ef Mon Sep 17 00:00:00 2001 From: Richard Date: Tue, 2 Jun 2020 22:22:34 -0400 Subject: [PATCH 12/15] Use of label now raises --- pandas/plotting/_matplotlib/hist.py | 10 +++++-- pandas/tests/plotting/test_groupby.py | 31 +++++++++++-------- pandas/tests/plotting/test_hist_method.py | 36 +++++++++++++++-------- 3 files changed, 49 insertions(+), 28 deletions(-) diff --git a/pandas/plotting/_matplotlib/hist.py b/pandas/plotting/_matplotlib/hist.py index 962f222676a72..ddb05990381de 100644 --- a/pandas/plotting/_matplotlib/hist.py +++ b/pandas/plotting/_matplotlib/hist.py @@ -251,8 +251,7 @@ def _grouped_hist( ------- collection of Matplotlib Axes """ - - if legend and "label" not in kwargs: + if legend: if isinstance(data, ABCDataFrame): if column is None: kwargs["label"] = data.columns @@ -308,6 +307,9 @@ def hist_series( ): import matplotlib.pyplot as plt + if legend and "label" in kwds: + raise ValueError("Cannot use both legend and label") + if by is None: if kwds.get("layout", None) is not None: raise ValueError("The 'layout' keyword is not supported when 'by' is None") @@ -322,7 +324,7 @@ def hist_series( elif ax.get_figure() != fig: raise AssertionError("passed axis not bound to passed figure") values = self.dropna().values - if legend and "label" not in kwds: + if legend: kwds["label"] = self.name ax.hist(values, bins=bins, **kwds) if legend: @@ -379,6 +381,8 @@ def hist_frame( legend: bool = False, **kwds, ): + if legend and "label" in kwds: + raise ValueError("Cannot use both legend and label") if by is not None: axes = _grouped_hist( data, diff --git a/pandas/tests/plotting/test_groupby.py b/pandas/tests/plotting/test_groupby.py index 1a5efc96c88f6..9ab851e330286 100644 --- a/pandas/tests/plotting/test_groupby.py +++ b/pandas/tests/plotting/test_groupby.py @@ -79,16 +79,19 @@ def test_groupby_hist_frame_with_legend(self, column, expected_axes_num, label): g = df.groupby("c") kwargs = {"legend": True, "column": column} - # We get warnings if kwargs contains "label": None + if label is not None: kwargs["label"] = label - - for axes in g.hist(**kwargs): - self._check_axes_shape( - axes, axes_num=expected_axes_num, layout=expected_layout - ) - for ax, expected_label in zip(axes[0], expected_labels): - self._check_legend_labels(ax, expected_label) + msg = "Cannot use both legend and label" + with pytest.raises(ValueError, match=msg): + g.hist(**kwargs) + else: + for axes in g.hist(**kwargs): + self._check_axes_shape( + axes, axes_num=expected_axes_num, layout=expected_layout + ) + for ax, expected_label in zip(axes[0], expected_labels): + self._check_legend_labels(ax, expected_label) @pytest.mark.parametrize( "label, expected_label", [(None, ["1", "2"]), ("d", ["d", "d"])] @@ -100,10 +103,14 @@ def test_groupby_hist_series_with_legend(self, label, expected_label): g = df.groupby("c") kwargs = {"legend": True} + # We get warnings if kwargs contains "label": None if label is not None: kwargs["label"] = label - - for ax in g["a"].hist(**kwargs): - self._check_axes_shape(ax, axes_num=1, layout=(1, 1)) - self._check_legend_labels(ax, expected_label) + msg = "Cannot use both legend and label" + with pytest.raises(ValueError, match=msg): + g.hist(**kwargs) + else: + for ax in g["a"].hist(**kwargs): + self._check_axes_shape(ax, axes_num=1, layout=(1, 1)) + self._check_legend_labels(ax, expected_label) diff --git a/pandas/tests/plotting/test_hist_method.py b/pandas/tests/plotting/test_hist_method.py index 5764f5e96423e..2764f4b52a610 100644 --- a/pandas/tests/plotting/test_hist_method.py +++ b/pandas/tests/plotting/test_hist_method.py @@ -142,13 +142,18 @@ def test_hist_with_legend( s.index.name = "b" kwargs = {"legend": True, "by": by} - # We get warnings if kwargs contains "label": None + if label is not None: kwargs["label"] = label - - axes = _check_plot_works(s.hist, **kwargs) - self._check_axes_shape(axes, axes_num=expected_axes_num, layout=expected_layout) - self._check_legend_labels(axes, expected_label) + msg = "Cannot use both legend and label" + with pytest.raises(ValueError, match=msg): + s.hist(**kwargs) + else: + axes = _check_plot_works(s.hist, **kwargs) + self._check_axes_shape( + axes, axes_num=expected_axes_num, layout=expected_layout + ) + self._check_legend_labels(axes, expected_label) @td.skip_if_no_mpl @@ -329,16 +334,21 @@ def test_hist_with_legend(self, by, column, label): df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"]) kwargs = {"legend": True, "by": by, "column": column} - # We get warnings if kwargs contains "label": None + if label is not None: kwargs["label"] = label - - axes = _check_plot_works(df.hist, **kwargs) - self._check_axes_shape(axes, axes_num=expected_axes_num, layout=expected_layout) - if by is None and column is None and label is None: - axes = axes[0] - for expected_label, ax in zip(expected_labels, axes): - self._check_legend_labels(ax, expected_label) + msg = "Cannot use both legend and label" + with pytest.raises(ValueError, match=msg): + df.hist(**kwargs) + else: + axes = _check_plot_works(df.hist, **kwargs) + self._check_axes_shape( + axes, axes_num=expected_axes_num, layout=expected_layout + ) + if by is None and column is None and label is None: + axes = axes[0] + for expected_label, ax in zip(expected_labels, axes): + self._check_legend_labels(ax, expected_label) @td.skip_if_no_mpl From 0dba5d9e4ca1a0ee5de80beab43436557173f838 Mon Sep 17 00:00:00 2001 From: Richard Date: Thu, 4 Jun 2020 18:28:05 -0400 Subject: [PATCH 13/15] Refactored tests, improved label logic in _grouped_hist --- pandas/plotting/_matplotlib/hist.py | 12 ++-- pandas/tests/plotting/test_groupby.py | 70 +++++++++++----------- pandas/tests/plotting/test_hist_method.py | 72 +++++++++++------------ 3 files changed, 74 insertions(+), 80 deletions(-) diff --git a/pandas/plotting/_matplotlib/hist.py b/pandas/plotting/_matplotlib/hist.py index ddb05990381de..ee41479b3c7c9 100644 --- a/pandas/plotting/_matplotlib/hist.py +++ b/pandas/plotting/_matplotlib/hist.py @@ -252,13 +252,13 @@ def _grouped_hist( collection of Matplotlib Axes """ if legend: - if isinstance(data, ABCDataFrame): - if column is None: - kwargs["label"] = data.columns - else: - kwargs["label"] = column - else: + assert "label" not in kwargs + if data.ndim == 1: kwargs["label"] = data.name + elif column is None: + kwargs["label"] = data.columns + else: + kwargs["label"] = column def plot_group(group, ax): ax.hist(group.dropna().values, bins=bins, **kwargs) diff --git a/pandas/tests/plotting/test_groupby.py b/pandas/tests/plotting/test_groupby.py index 9ab851e330286..4ac23c2cffa15 100644 --- a/pandas/tests/plotting/test_groupby.py +++ b/pandas/tests/plotting/test_groupby.py @@ -68,49 +68,47 @@ def test_plot_kwargs(self): assert len(res["a"].collections) == 1 @pytest.mark.parametrize("column, expected_axes_num", [(None, 2), ("b", 1)]) - @pytest.mark.parametrize("label", [None, "d"]) - def test_groupby_hist_frame_with_legend(self, column, expected_axes_num, label): - # GH 6279 - Histogram can have a legend + def test_groupby_hist_frame_with_legend(self, column, expected_axes_num): + # GH 6279 - DataFrameGroupBy histogram can have a legend expected_layout = (1, expected_axes_num) - expected_labels = label or column or [["a"], ["b"]] + expected_labels = column or [["a"], ["b"]] index = Index(15 * ["1"] + 15 * ["2"], name="c") df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"]) g = df.groupby("c") - kwargs = {"legend": True, "column": column} - - if label is not None: - kwargs["label"] = label - msg = "Cannot use both legend and label" - with pytest.raises(ValueError, match=msg): - g.hist(**kwargs) - else: - for axes in g.hist(**kwargs): - self._check_axes_shape( - axes, axes_num=expected_axes_num, layout=expected_layout - ) - for ax, expected_label in zip(axes[0], expected_labels): - self._check_legend_labels(ax, expected_label) - - @pytest.mark.parametrize( - "label, expected_label", [(None, ["1", "2"]), ("d", ["d", "d"])] - ) - def test_groupby_hist_series_with_legend(self, label, expected_label): - # GH 6279 - Histogram can have a legend + for axes in g.hist(legend=True, column=column): + self._check_axes_shape( + axes, axes_num=expected_axes_num, layout=expected_layout + ) + for ax, expected_label in zip(axes[0], expected_labels): + self._check_legend_labels(ax, expected_label) + + @pytest.mark.parametrize("column", [None, "b"]) + def test_groupby_hist_frame_with_legend_raises(self, column): + # GH 6279 - DataFrameGroupBy histogram with legend and label raises index = Index(15 * ["1"] + 15 * ["2"], name="c") df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"]) g = df.groupby("c") - kwargs = {"legend": True} - - # We get warnings if kwargs contains "label": None - if label is not None: - kwargs["label"] = label - msg = "Cannot use both legend and label" - with pytest.raises(ValueError, match=msg): - g.hist(**kwargs) - else: - for ax in g["a"].hist(**kwargs): - self._check_axes_shape(ax, axes_num=1, layout=(1, 1)) - self._check_legend_labels(ax, expected_label) + with pytest.raises(ValueError, match="Cannot use both legend and label"): + g.hist(legend=True, column=column, label="d") + + def test_groupby_hist_series_with_legend(self): + # GH 6279 - SeriesGroupBy histogram can have a legend + index = Index(15 * ["1"] + 15 * ["2"], name="c") + df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"]) + g = df.groupby("c") + + for ax in g["a"].hist(legend=True): + self._check_axes_shape(ax, axes_num=1, layout=(1, 1)) + self._check_legend_labels(ax, ["1", "2"]) + + def test_groupby_hist_series_with_legend_raises(self): + # GH 6279 - SeriesGroupBy histogram with legend and label raises + index = Index(15 * ["1"] + 15 * ["2"], name="c") + df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"]) + g = df.groupby("c") + + with pytest.raises(ValueError, match="Cannot use both legend and label"): + g.hist(legend=True, label="d") diff --git a/pandas/tests/plotting/test_hist_method.py b/pandas/tests/plotting/test_hist_method.py index 2764f4b52a610..7644a54979a9a 100644 --- a/pandas/tests/plotting/test_hist_method.py +++ b/pandas/tests/plotting/test_hist_method.py @@ -132,28 +132,25 @@ def test_plot_fails_when_ax_differs_from_figure(self): @pytest.mark.parametrize( "by, expected_axes_num, expected_layout", [(None, 1, (1, 1)), ("b", 2, (1, 2))] ) - @pytest.mark.parametrize("label, expected_label", [(None, "a"), ("c", "c")]) - def test_hist_with_legend( - self, by, expected_axes_num, expected_layout, label, expected_label - ): - # GH 6279 - Histogram can have a legend + def test_hist_with_legend(self, by, expected_axes_num, expected_layout): + # GH 6279 - Series histogram can have a legend index = 15 * ["1"] + 15 * ["2"] s = Series(np.random.randn(30), index=index, name="a") s.index.name = "b" - kwargs = {"legend": True, "by": by} - - if label is not None: - kwargs["label"] = label - msg = "Cannot use both legend and label" - with pytest.raises(ValueError, match=msg): - s.hist(**kwargs) - else: - axes = _check_plot_works(s.hist, **kwargs) - self._check_axes_shape( - axes, axes_num=expected_axes_num, layout=expected_layout - ) - self._check_legend_labels(axes, expected_label) + axes = _check_plot_works(s.hist, legend=True, by=by) + self._check_axes_shape(axes, axes_num=expected_axes_num, layout=expected_layout) + self._check_legend_labels(axes, "a") + + @pytest.mark.parametrize("by", [None, "b"]) + def test_hist_with_legend_raises(self, by): + # GH 6279 - Series histogram with legend and label raises + index = 15 * ["1"] + 15 * ["2"] + s = Series(np.random.randn(30), index=index, name="a") + s.index.name = "b" + + with pytest.raises(ValueError, match="Cannot use both legend and label"): + s.hist(legend=True, by=by, label="c") @td.skip_if_no_mpl @@ -321,34 +318,33 @@ def test_hist_column_order_unchanged(self, column, expected): @pytest.mark.parametrize("by", [None, "c"]) @pytest.mark.parametrize("column", [None, "b"]) - @pytest.mark.parametrize("label", [None, "d"]) - def test_hist_with_legend(self, by, column, label): - # GH 6279 - Histogram can have a legend + def test_hist_with_legend(self, by, column): + # GH 6279 - DataFrame histogram can have a legend expected_axes_num = 1 if by is None and column is not None else 2 expected_layout = (1, expected_axes_num) - expected_labels = label or column or ["a", "b"] + expected_labels = column or ["a", "b"] if by is not None: expected_labels = [expected_labels] * 2 index = Index(15 * ["1"] + 15 * ["2"], name="c") df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"]) - kwargs = {"legend": True, "by": by, "column": column} - - if label is not None: - kwargs["label"] = label - msg = "Cannot use both legend and label" - with pytest.raises(ValueError, match=msg): - df.hist(**kwargs) - else: - axes = _check_plot_works(df.hist, **kwargs) - self._check_axes_shape( - axes, axes_num=expected_axes_num, layout=expected_layout - ) - if by is None and column is None and label is None: - axes = axes[0] - for expected_label, ax in zip(expected_labels, axes): - self._check_legend_labels(ax, expected_label) + axes = _check_plot_works(df.hist, legend=True, by=by, column=column) + self._check_axes_shape(axes, axes_num=expected_axes_num, layout=expected_layout) + if by is None and column is None: + axes = axes[0] + for expected_label, ax in zip(expected_labels, axes): + self._check_legend_labels(ax, expected_label) + + @pytest.mark.parametrize("by", [None, "c"]) + @pytest.mark.parametrize("column", [None, "b"]) + def test_hist_with_legend_raises(self, by, column): + # GH 6279 - DataFrame histogram with legend and label raises + index = Index(15 * ["1"] + 15 * ["2"], name="c") + df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"]) + + with pytest.raises(ValueError, match="Cannot use both legend and label"): + df.hist(legend=True, by=by, column=column, label="d") @td.skip_if_no_mpl From 1c9cef595029e3fa7373c5b83f2c20e3b0e29a22 Mon Sep 17 00:00:00 2001 From: Richard Date: Thu, 4 Jun 2020 21:43:52 -0400 Subject: [PATCH 14/15] Moved legend kwarg behind backend --- pandas/plotting/_core.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/pandas/plotting/_core.py b/pandas/plotting/_core.py index 6936a483aa7ff..4f5b7b2d7a888 100644 --- a/pandas/plotting/_core.py +++ b/pandas/plotting/_core.py @@ -26,8 +26,8 @@ def hist_series( yrot: Optional[float] = None, figsize: Optional[Tuple[int, int]] = None, bins: Union[int, Sequence[int]] = 10, - legend: bool = False, backend: Optional[str] = None, + legend: bool = False, **kwargs, ): """ @@ -56,11 +56,6 @@ def hist_series( bin edges are calculated and returned. If bins is a sequence, gives bin edges, including left edge of first bin and right edge of last bin. In this case, bins is returned unmodified. - legend : bool, default False - Whether to show the legend. - - ..versionadded:: 1.1.0 - backend : str, default None Backend to use instead of the backend specified in the option ``plotting.backend``. For instance, 'matplotlib'. Alternatively, to @@ -69,6 +64,11 @@ def hist_series( .. versionadded:: 1.0.0 + legend : bool, default False + Whether to show the legend. + + ..versionadded:: 1.1.0 + **kwargs To be passed to the actual plotting function. @@ -113,8 +113,8 @@ def hist_frame( figsize: Optional[Tuple[int, int]] = None, layout: Optional[Tuple[int, int]] = None, bins: Union[int, Sequence[int]] = 10, - legend: bool = False, backend: Optional[str] = None, + legend: bool = False, **kwargs, ): """ @@ -167,10 +167,6 @@ def hist_frame( bin edges are calculated and returned. If bins is a sequence, gives bin edges, including left edge of first bin and right edge of last bin. In this case, bins is returned unmodified. - legend : bool, default False - Whether to show the legend. - - ..versionadded:: 1.1.0 backend : str, default None Backend to use instead of the backend specified in the option @@ -180,6 +176,11 @@ def hist_frame( .. versionadded:: 1.0.0 + legend : bool, default False + Whether to show the legend. + + ..versionadded:: 1.1.0 + **kwargs All other plotting keyword arguments to be passed to :meth:`matplotlib.pyplot.hist`. From 11cf3f8d7e45ff8349fc694e7d3570a819cd933b Mon Sep 17 00:00:00 2001 From: Richard Date: Wed, 10 Jun 2020 17:06:13 -0400 Subject: [PATCH 15/15] Fixed whatsnew --- doc/source/whatsnew/v1.1.0.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v1.1.0.rst b/doc/source/whatsnew/v1.1.0.rst index d63572b04182f..a60c55c26a7ec 100644 --- a/doc/source/whatsnew/v1.1.0.rst +++ b/doc/source/whatsnew/v1.1.0.rst @@ -291,7 +291,7 @@ Other enhancements - :meth:`groupby.transform` now allows ``func`` to be ``pad``, ``backfill`` and ``cumcount`` (:issue:`31269`). - :meth:`~pandas.io.json.read_json` now accepts `nrows` parameter. (:issue:`33916`). - :meth `~pandas.io.gbq.read_gbq` now allows to disable progress bar (:issue:`33360`). -- :meth:`DataFrame.hist`, :meth:`Series.hist`, :meth:`DataFrameGroupby.hist`, and :meth:`SeriesGroupby.hist` have gained the ``legend`` argument. Set to True to show a legend in the histogram. (:issue:`6279`) +- :meth:`DataFrame.hist`, :meth:`Series.hist`, :meth:`core.groupby.DataFrameGroupBy.hist`, and :meth:`core.groupby.SeriesGroupBy.hist` have gained the ``legend`` argument. Set to True to show a legend in the histogram. (:issue:`6279`) .. ---------------------------------------------------------------------------