-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
BUG: groupby.hist legend should use group keys #33493
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 19 commits
bab7491
e0c9466
61693c6
cbfc167
a5f5ba1
726d147
3e4925d
8db5247
a46609a
c36cae2
c01c7ab
c207ca2
a1a7e27
5b9cae7
1db3b35
0b0dbdd
0dba5d9
4f24547
1c9cef5
11cf3f8
c5749cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -225,6 +225,7 @@ def _grouped_hist( | |
xrot=None, | ||
ylabelsize=None, | ||
yrot=None, | ||
legend=False, | ||
**kwargs, | ||
): | ||
""" | ||
|
@@ -243,15 +244,26 @@ 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 | ||
------- | ||
collection of Matplotlib Axes | ||
""" | ||
if legend: | ||
assert "label" not in kwargs | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is pandas the only one calling this, or could users call it with labels in kwargs? Users shouldn't see bare asserts like this. It should be a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And just to verify the expected behavior, if the user provides a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the first question, this function is only called from hist_frame and hist_series, each of which raise a ValueError on this condition. This assert is to maintain this behavior if in the future any other code paths call it. For the second question, see #33493 (comment) |
||
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) | ||
if legend: | ||
ax.legend() | ||
|
||
if xrot is None: | ||
xrot = rot | ||
|
@@ -290,10 +302,14 @@ def hist_series( | |
yrot=None, | ||
figsize=None, | ||
bins=10, | ||
legend: bool = False, | ||
**kwds, | ||
): | ||
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") | ||
|
@@ -308,8 +324,11 @@ def hist_series( | |
elif ax.get_figure() != fig: | ||
raise AssertionError("passed axis not bound to passed figure") | ||
values = self.dropna().values | ||
|
||
if legend: | ||
kwds["label"] = self.name | ||
ax.hist(values, bins=bins, **kwds) | ||
if legend: | ||
ax.legend() | ||
ax.grid(grid) | ||
axes = np.array([ax]) | ||
|
||
|
@@ -334,6 +353,7 @@ def hist_series( | |
xrot=xrot, | ||
ylabelsize=ylabelsize, | ||
yrot=yrot, | ||
legend=legend, | ||
**kwds, | ||
) | ||
|
||
|
@@ -358,8 +378,11 @@ def hist_frame( | |
figsize=None, | ||
layout=None, | ||
bins=10, | ||
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, | ||
|
@@ -376,6 +399,7 @@ def hist_frame( | |
xrot=xrot, | ||
ylabelsize=ylabelsize, | ||
yrot=yrot, | ||
legend=legend, | ||
**kwds, | ||
) | ||
return axes | ||
|
@@ -401,11 +425,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 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DataFrameGroupBy and SeriesGroupBy aren't in the top-level. It should be something like
core.groupby.DataFrameGroupBy
. Seedoc/source/referenece.groupby.rst
for the right path..There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah - thanks!