BUG: groupby.hist legend should use group keys
State: closed

- [x] closes #6279
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

cc @TomAugspurger 

Added argument "legend" to histogram backend. This adds the ability to display a legend even when not using a groupby. 

````
df = pd.DataFrame(np.random.randn(30, 2), columns=['A', 'B'])
df['C'] = 15 * ['a'] + 15 * ['b']
df = df.set_index('C')
df.groupby('C')['A'].hist(legend=True)
````

produces

![image](https://user-images.githubusercontent.com/45562402/79070334-25911500-7ca3-11ea-8303-d44ded966137.png)

I went off the tests that already existed in test_hist_method and test_groupby, but perhaps more stringent tests should be added. I've been manually checking with the following code:

````
import matplotlib.pyplot as plt
import pandas as pd

df = pd.DataFrame(np.random.randn(30, 2), columns=['A', 'B'])
df['C'] = 15 * ['a'] + 15 * ['b']
df = df.set_index('C')

df.groupby('C')['A'].hist(legend=False)
plt.show()
df.groupby('C')['A'].hist(legend=True)
plt.show()
df.A.hist(by='C', legend=False)
plt.show()
df.A.hist(by='C', legend=True)
plt.show()
plt.show()
df.hist(by='C', legend=False)
plt.show()
df.hist(by='C', legend=True)
plt.show()
df.hist(by='C', column='B', legend=False)
plt.show()
df.hist(by='C', column='B', legend=True)
plt.show()

df.groupby('C')['A'].hist(label='D', legend=False)
plt.show()
df.groupby('C')['A'].hist(label='D', legend=True)
plt.show()
df.A.hist(by='C', label='D', legend=False)
plt.show()
df.A.hist(by='C', label='D', legend=True)
plt.show()
df.hist(by='C', label='D', legend=False)
plt.show()
df.hist(by='C', label='D', legend=True)
plt.show()
df.hist(by='C', column='B', label='D', legend=False)
plt.show()
df.hist(by='C', column='B', label='D', legend=True)
plt.show()
````

Labels:
name: Visualization
description: plotting

Comments:
Thanks for the PR!

@rhshadrach you still interested in working on this? Mind trying to fix up the tests if so?
@alimcmaster1 Yep - this is still active, thanks for the response. I'd be happy to improve the tests, but I'm not sure how. Do you have any recommendations?
@alimcmaster1 I've improved the tests in test_hist_method. test_groupby on the other hand seems to only make sure that the groupby method works. I'm guessing the idea is to make sure groupby works there, and then leave the details to the other plotting tests. With this, I've left that test as-is (aside from minor formatting changes). But let me know if you think that test should be improved somehow.
The introduced test fails on an older version of matplotlib. The exact line it fails on is

    labels = [six.text_type(lab) for lab in label]

This issue is fixed in matplotlib master by converting the label to a string
   
     labels = [] if label is None else np.atleast_1d(np.asarray(label, str))

with this, I feel pretty comfortable changing the test labels to string to get them to pass.
@charlesdong1991 Thanks for the review, changes made and checks pass. For the whatsnew, it seemed to me to be a toss up as whether this is a bug or enhancement; I went with enhancement. I also added one additional test for SeriesGroupby.hist.
@WillAyd, @charlesdong1991 Thanks for the reviews - changes made and checks pass.
nice job, lgtm

cc @jreback @WillAyd 
@WillAyd changes made, use of label and legend now raises.
Yep that’s what I’m referring to

Sent from my iPhone

> On Jun 4, 2020, at 3:02 PM, rhshadrach <notifications@github.com> wrote:
> 
> ﻿
> @rhshadrach commented on this pull request.
> 
> In pandas/plotting/_matplotlib/hist.py:
> 
> >      kwargs : dict, keyword arguments passed to matplotlib.Axes.hist
>  
>      Returns
>      -------
>      collection of Matplotlib Axes
>      """
> +    if legend:
> +        if isinstance(data, ABCDataFrame):
> +            if column is None:
> +                kwargs["label"] = data.columns
> +            else:
> When you say branch - do you mean just the else clause?
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or unsubscribe.

@WillAyd Changes made and checks pass. In addition to the splitting out the tests, I also improve the logic involving setting the label. Instead of using isinstance, it now uses ndim. Also the logic is flat instead of nested.
> One question about the expected behavior though: why don't we use a user-provided `label` when `legend=True`?

Talked about this starting here: https://github.com/pandas-dev/pandas/pull/33493#discussion_r430851716 so I specifically asked to have it raise. Happy to be overridden if you think better otherwise
@TomAugspurger: I've fixed the issues you pointed out in the whatsnew. What's still uncertain is how to handle label when legend=True. From https://github.com/pandas-dev/pandas/pull/33493#discussion_r430851716, I was for supporting and not overwriting label whereas @WillAyd was opposed to supporting because there was another way to set the labels (namely, changing the column names of the dataframe). Would like to get your thoughts here.
@TomAugspurger friendly ping. Just want to let you know I'll be unavailable starting the 24th until July 5th in case we want to get this into 1.1. No issue with it waiting until 1.2 on my end if the timing doesn't work out.
Don't have strong thoughts. Let's just keep it as on this PR and adjust based on user feedback if it arises.
Thanks @rhshadrach